Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback to opening options page manually in brave #315

Merged
merged 2 commits into from
Nov 27, 2017
Merged

Conversation

olizilla
Copy link
Member

@olizilla olizilla commented Nov 17, 2017

In brave the runtime.openOptionsPage() call is failing, so the user can't update their prefs.

See: brave/browser-laptop#7812

This PR adds a fallback to opening the options page as a browser tab if the runtime.openOptionsPage() throws.

screenshot 2017-11-17 16 13 59

Also, we add try/catch around calls the following currently unsupported apis

  • contextMenus.create
  • contextMenus.update

In the meantime I'll follow up with brave to see what we need to do to make them available.

WIP on #312

@lidel
Copy link
Member

lidel commented Nov 17, 2017

LGTM, but lets keep it open until #311 is merged.

@lidel
Copy link
Member

lidel commented Nov 26, 2017

@olizilla now that #311 is merged, feel free to rebase this PR against master so that we can merge it too.

@olizilla olizilla changed the title Fallback to opening options page manually in brave [WIP] Fallback to opening options page manually in brave Nov 27, 2017
olizilla and others added 2 commits November 27, 2017 13:04
- Update options page fallback with path to dist
- Dial down non-fatal error reporting. We already caught it.
@olizilla olizilla changed the title [WIP] Fallback to opening options page manually in brave Fallback to opening options page manually in brave Nov 27, 2017
@olizilla
Copy link
Member Author

@lidel all fixed up! We still don't get to see the browserAction icon in brave yet which is annoying but this PR gets the basics working in brave.

@lidel lidel merged commit 7814ab1 into master Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants