Skip to content
This repository was archived by the owner on May 12, 2022. It is now read-only.

Comments

Fix manifest creation#12

Open
MakMuftic wants to merge 4 commits intoMetaMask:developfrom
ChainSafe:mmuftic/fix-manifest-creation
Open

Fix manifest creation#12
MakMuftic wants to merge 4 commits intoMetaMask:developfrom
ChainSafe:mmuftic/fix-manifest-creation

Conversation

@MakMuftic
Copy link

@MakMuftic MakMuftic commented Oct 4, 2021

Fixes: #11

Explanation:
I tracked a bug inside Metamasks code that creates browser-specific manifests. The idea here is to have _base.json manifest that holds manifest properties that are needed on all browsers and then also have browser-specific manifests such as chrome.json. Then, when manifest for specific browser distribution is created, these two manifests are merged into one. The problem occurs when you have array property such as permissions in both _base.json and chrome.json (browser-specific manifest). Below you can find examples of how this merge worked before, and how it works now.

OLD BEHAVIOUR

  /* _base.json */
  "permissions": [
    "storage",
    "unlimitedStorage",
    "clipboardWrite",
    "http://localhost:8545/",
    "https://*.infura.io/",
    "activeTab",
    "webRequest",
    "*://*.eth/",
    "notifications"
  ]
  
  /* chrome.json */
  "permissions": [
    "chrome-extension://*/*"
  ]
  
  /* results in merged permissions as shown below */
  /* problem is that new permissions from browser specific manifest ovveride base permission /*
  "permissions": [
    "chrome-extension://*/*",  /* notice that storage permission is missing */
    "unlimitedStorage",
    "clipboardWrite",
    "http://localhost:8545/",
    "https://*.infura.io/",
    "activeTab",
    "webRequest",
    "*://*.eth/",
    "notifications"
  ]
  

NEW BEHAVIOUR

  /* _base.json */
  "permissions": [
    "storage",
    "unlimitedStorage",
    "clipboardWrite",
    "http://localhost:8545/",
    "https://*.infura.io/",
    "activeTab",
    "webRequest",
    "*://*.eth/",
    "notifications"
  ]
  
  /* chrome.json */
  "permissions": [
    "chrome-extension://*/*"
  ]
  
  /* results in merged permissions as shown below */
  "permissions": [ 
    "storage"
    "unlimitedStorage",
    "clipboardWrite",
    "http://localhost:8545/",
    "https://*.infura.io/",
    "activeTab",
    "webRequest",
    "*://*.eth/",
    "notifications",
    "chrome-extension://*/*"
  ]
  

I have read the CLA Document and I hereby sign the CLA

@MakMuftic MakMuftic requested review from a team and kumavis as code owners October 4, 2021 14:27
@MakMuftic MakMuftic requested a review from danjm October 4, 2021 14:27
@MakMuftic
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

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.

Fix browser-specific manifest generation

1 participant