Skip to content

Fix getting the site metadata from the request origin params.#10243

Merged
rekmarks merged 2 commits intodevelopfrom
fix-getSiteMetadata
Jan 22, 2021
Merged

Fix getting the site metadata from the request origin params.#10243
rekmarks merged 2 commits intodevelopfrom
fix-getSiteMetadata

Conversation

@tmashuang
Copy link
Copy Markdown
Contributor

Regressed from inpage-provider ts migration the request. The property domainMetadata is now set as a params key for the request.

https://github.com/MetaMask/inpage-provider/blob/v7.0.0/src/siteMetadata.js#L19-L25
vs
https://github.com/MetaMask/inpage-provider/blob/main/src/siteMetadata.ts#L19-L27

Fixes getting the site metadata to populate the domainMetadata in state that is used for populating the decrypt message component which has been broken since the updated of the inpage-provider package.

@tmashuang tmashuang requested a review from a team as a code owner January 21, 2021 23:38
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7bd1be0]
Page Load Metrics (551 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint389357136
domContentLoaded32668055012359
load32768155112359
domInteractive32668055012359

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, great catch!

Fixes getting the site metadata to populate the domainMetadata in state that is used for populating the decrypt message component which has been broken since the updated of the inpage-provider package.

How broken is it exactly? Does it crash, or is it just the site icon that's missing? If it's worse than a missing icon, we should also update the UI to be more tolerant of missing site metadata. Sites/extensions with embedded providers might fail to send site metadata, so we shouldn't assume it's present anywhere if at all possible.

@danfinlay
Copy link
Copy Markdown
Contributor

Do you know if this would fix #10247?

@rekmarks rekmarks merged commit 1dad4ab into develop Jan 22, 2021
@rekmarks rekmarks deleted the fix-getSiteMetadata branch January 22, 2021 18:27
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants