fix/consistent-browser-api-usage#545
Open
PhilixTheExplorer wants to merge 1 commit intofossasia:mainfrom
Open
Conversation
…-browser compatibility
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR standardizes extension code to use the Promise-based Sequence diagram for updated shortcut notification in popupsequenceDiagram
actor User
participant PopupScript
participant BrowserAPI
User->>PopupScript: triggerShortcutNotification
PopupScript->>PopupScript: showShortcutNotification(messageKey)
PopupScript->>BrowserAPI: check browser_i18n
alt browser_i18n_unavailable
PopupScript-->>User: no_notification
else browser_i18n_available
PopupScript->>BrowserAPI: i18n_getMessage(messageKey)
BrowserAPI-->>PopupScript: localizedMessage
PopupScript->>PopupScript: createOrUpdateNotification(localizedMessage)
PopupScript-->>User: notificationDisplayed
end
Flow diagram for storage normalization in allIncludedflowchart TD
Start[Start allIncluded]
Load[Load_settings_from_browser_storage]
CheckBoth[onlyIssues && onlyPRs?]
NormalizeOnlyPRs[Set onlyPRs = false]
PersistOnlyPRs[browser_storage_local_set onlyPRs_false]
CheckMerged[onlyMergedPRs_enabled?]
BuildCorrections[Build_corrections_for_onlyRevPRs_onlyIssues_onlyPRs]
PersistCorrections[browser_storage_local_set corrections]
Continue[Continue_allIncluded_processing]
Start --> Load --> CheckBoth
CheckBoth -- Yes --> NormalizeOnlyPRs --> PersistOnlyPRs --> CheckMerged
CheckBoth -- No --> CheckMerged
CheckMerged -- Yes --> BuildCorrections --> PersistCorrections --> Continue
CheckMerged -- No --> Continue
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR resolves Firefox compatibility and codebase consistency issues by replacing remaining direct chrome.* extension API usages with browser.*, aligning with the repo’s use of browser-polyfill.min.js.
Changes:
- Updated
scrumHelper.jsto usebrowser.i18n.getMessageandbrowser.storage.local.setinstead ofchrome.*. - Updated
popup.jsto usebrowser.i18nfor shortcut notification rendering and guarding.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/scripts/scrumHelper.js | Switches remaining chrome.* i18n/storage calls to browser.* for promise-based, cross-browser behavior. |
| src/scripts/popup.js | Updates shortcut notification i18n guard + message retrieval to use browser.*. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Fixes
Fixes #542
📝 Summary of Changes
chrome.*withbrowser.*inscrumHelper.js:chrome.i18n→browser.i18n(inrenderErrorMessage())chrome.storage.local.set()→browser.storage.local.set()(onlyIssues/onlyPRs mutual exclusivity fix)chrome.storage.local.set()→browser.storage.local.set()(onlyMergedPRs corrections)chrome.*withbrowser.*inpopup.js:chrome.i18nguard →browser.i18n(inshowShortcutNotification())chrome.i18n.getMessage()→browser.i18n.getMessage()The project uses
browser-polyfill.min.jsfor cross-browser support. The entire codebaseconsistently uses
browser.*APIs — except these 5 places that usedchrome.*directly.While this works in Chrome (where
chrome.*is native), in Firefoxchrome.storage.local.set()uses callbacks instead of Promises, which can cause silent failures.
How to verify
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Ensure consistent use of WebExtension browser APIs for cross-browser compatibility.
Bug Fixes:
Enhancements: