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

Firefox fixes #209

Merged
merged 7 commits into from
Aug 19, 2023
Merged

Firefox fixes #209

merged 7 commits into from
Aug 19, 2023

Conversation

praschke
Copy link
Collaborator

@praschke praschke commented Aug 16, 2023

  • the readme and CSP wasn't changed when Kibana's handlebars was added to the repo.
  • FF 102 moved script and style injection to the scripting API
  • instead of the Chrome-only StyleOrigin enum, string literals may be used
  • window.getSelection() can return null on FF, throwing some noisy errors
  • there's something wrong with declarativeNetRequest on FF that breaks after the browser restarts.

Fixes #173. Improves #96. The remaining lint warnings on FF pertain to storage.session and eval in Handlebars and WanaKana.

@djahandarie
Copy link
Collaborator

@praschke Very nice! Thank you. This fixes a ton of small problems and gets rid of MV2 / fallback cruft.

One question: what is declarativeNetRequest used for in the first place? I imagine it was in there for a reason, so I'm trying to figure out what we would need to track. Maybe we could open a separate issue to track fixing whatever functionality is enabled by that permission on FF?

@github-actions
Copy link

github-actions bot commented Aug 16, 2023

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@praschke
Copy link
Collaborator Author

One question: what is declarativeNetRequest used for in the first place? I imagine it was in there for a reason, so I'm trying to figure out what we would need to track. Maybe we could open a separate issue to track fixing whatever functionality is enabled by that permission on FF?

declarativeNetRequest is what Chrome is forcing extensions to use with MV3. Previously webRequest was available in Chrome. Firefox is maintaining the availability of webRequest because if uBlock Origin got gimped they'd probably lose the last of their users.

declarativeNetRequest and webRequest are used in request-builder.js to make audio requests. The existing logic with webRequest is still present, but if declarativeNetRequest is detected then it uses that instead. No functionality is lost.

@djahandarie
Copy link
Collaborator

Got it, makes sense, thank you for elaborating! In that case LGTM!

djahandarie
djahandarie previously approved these changes Aug 16, 2023
Firefox added the scripting API in 102. This should fix the majority
of warnings listed in yomidevs#96:

- insertCSS
- executeScript
- getRegisteredContentScripts
- contentScripts.register
- registerContentScripts
- unregisterContentScripts
the API accepts string literals, which is all this enum provides. This
should fix two warnings in yomidevs#96.
`declarativeNetRequest.updateDynamicRules()` returns with an
unexpected error in Firefox, but only after the browser has been
restarted. On a fresh install of Yomitan it works, causing bug
flakiness. `declarativeNetRequest` can be disabled in the manifest as
a workaround.
this line serves no purpose. the commit it was introduced in has the
message 'Document RequestBuilder' and is the only non-documentary line
in the commit.

related to yomidevs#204.
@djahandarie djahandarie added kind/bug The issue or PR is regarding a bug browser/firefox The issue is Firefox-only labels Aug 19, 2023
@djahandarie djahandarie merged commit a00a56c into yomidevs:master Aug 19, 2023
3 checks passed
@praschke praschke deleted the firefox-fixes branch October 1, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser/firefox The issue is Firefox-only kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom URL (Json) not working on Firefox
2 participants