Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix extension icons appearing in about: pages and context menu #12412

Merged
merged 1 commit into from
Dec 27, 2017

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Dec 27, 2017

fix #12409

Test Plan:

  1. go to about:preferences#extensions, all icons should appear.
  2. enable pocket. the browserAction icon should appear in the top right of the browser.
  3. go to about:extensions. all icons should appear.
  4. go to any page and right-click. the pocket icon should appear in the context menu.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

fix #12409

Test Plan:
1. go to about:preferences#extensions, all icons should appear.
2. enable pocket. the browserAction icon should appear in the top right of the browser.
3. go to about:extensions. all icons should appear.
4. go to any page and right-click. the pocket icon should appear in the context menu.
@@ -408,7 +408,7 @@ module.exports.init = () => {
extensionInfo.setInstallInfo(installInfo.id, installInfo)
installInfo.filePath = installInfo.base_path

installInfo.base_path = chromeUrl(installInfo.base_path)
installInfo.base_path = fileUrl(installInfo.base_path)
Copy link
Member Author

Choose a reason for hiding this comment

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

@kevinlawler i un-did this change from a51b7ea because chrome://brave URLs are not loadable in about: pages and also the code that generated context menu icons assumed a file:// URL. if i'm not mistaken, chrome://brave is only needed for the browser action.

Copy link
Member

@bsclifton bsclifton Dec 27, 2017

Choose a reason for hiding this comment

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

@diracdeltas with @kevinlawler 's PR before fixed the icons being broken in the top right (part of the UI Chrome). I wonder if we need to store both the Chrome URL + file URL?

Copy link
Member

@bsclifton bsclifton Dec 27, 2017

Choose a reason for hiding this comment

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

I just verified that with this change, the icons still show up as expected 😄 I now see the change you did by wrapping browserAction! 👍

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Works great 😄 👍

@bsclifton bsclifton merged commit d0c2c6c into master Dec 27, 2017
@bsclifton bsclifton deleted the fix/chrome-csp branch December 27, 2017 18:38
bsclifton added a commit that referenced this pull request Dec 27, 2017
Fix extension icons appearing in about: pages and context menu
bsclifton added a commit that referenced this pull request Dec 27, 2017
Fix extension icons appearing in about: pages and context menu
@bsclifton
Copy link
Member

master d0c2c6c
0.21.x d612012
0.20.x 5614705

@bsclifton bsclifton added this to the 0.20.x (Beta Channel) milestone Dec 27, 2017
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.

Extensions icon broken on about:extensions
2 participants