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.

Sign up for GitHub

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

Jump to bottom

Fix MV3 on Firefox for Android #7464

Merged
merged 8 commits into from
May 29, 2024
Merged

Fix MV3 on Firefox for Android #7464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
93a47f9
Fix on FIrefox for Android
Samq64 May 24, 2024
16d09d4
Format code
Samq64 May 24, 2024
a68164c
Just use manifest icons when unmuting
Samq64 May 25, 2024
980195c
Fix double chrome
Samq64 May 25, 2024
62c3750
Check if chrome.contextMenus is undefined directly
Samq64 May 25, 2024
2b9eae0
More fixes
Samq64 May 25, 2024
fab727d
Use optional chaining (`?.`) when possible
WorldLanguages May 25, 2024
2ad25e9
Add code comment
WorldLanguages May 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions background/handle-notifications.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ chrome.storage.local.get("muted", (obj) => {
scratchAddons.muted = obj.muted;
});

chrome.contextMenus.removeAll();
chrome.contextMenus?.removeAll();
let currentMenuItem = null;

chrome.contextMenus.onClicked.addListener(({ parentMenuItemId, menuItemId }) => {
// NOTE: chrome.contextMenus equals `undefined` on Firefox for Android!

chrome.contextMenus?.onClicked.addListener(({ parentMenuItemId, menuItemId }) => {
if (parentMenuItemId === "mute") {
const mins = Number(menuItemId.split("_")[1]);
contextMenuMuted();
Expand All @@ -47,6 +49,7 @@ chrome.contextMenus.onClicked.addListener(({ parentMenuItemId, menuItemId }) =>
});

function contextMenuUnmuted() {
if (chrome.contextMenus === undefined) return; // Firefox for Android
if (currentMenuItem === "unmute") chrome.contextMenus.remove("unmute");
currentMenuItem = "mute";
chrome.contextMenus.create({
Expand All @@ -62,17 +65,18 @@ function contextMenuUnmuted() {
contexts: [BROWSER_ACTION],
});
}
// This seems to be run when the extension is loaded, so we'll just set the right icon here.
const prerelease = chrome.runtime.getManifest().version_name.includes("-prerelease");
chrome.browserAction.setIcon({
path: {
16: prerelease ? chrome.runtime.getURL("images/icon-blue-16.png") : chrome.runtime.getURL("images/icon-16.png"),
32: prerelease ? chrome.runtime.getURL("images/icon-blue-32.png") : chrome.runtime.getURL("images/icon-32.png"),
16: chrome.runtime.getURL(chrome.runtime.getManifest().icons["16"]),
32: chrome.runtime.getURL(chrome.runtime.getManifest().icons["32"]),
},
});
}

function contextMenuMuted() {
if (chrome.contextMenus === undefined) return; // Firefox for Android
// Note: in theory, this function is unreachable
// in FF for Android, but we early-return anyway.
if (currentMenuItem === "mute") chrome.contextMenus.remove("mute");
currentMenuItem = "unmute";
chrome.contextMenus.create({
Expand Down
1 change: 1 addition & 0 deletions webpages/popup/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
Copy link
Member

Choose a reason for hiding this comment

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

Could this cause some issues on desktop? 🤔 The size of the popup has frequently created problems.
Maybe to be on the safe side we should add this dynamically with JavaScript if the user agent matches mobile Chromium or mobile Firefox?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't noticed any problems so far and Vue 3 also adds it but it's better to be safe I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Unless it's handled differently in extension popups for some reason, the viewport tag has no effect on desktop.

Copy link
Member

Choose a reason for hiding this comment

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

Then let's assume this didn't break anything unless we've got proof otherwise

<script src="../set-lang.js" type="module"></script>
<script src="../check-unsupported.js" defer></script>
<style>
Expand Down
Loading