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

Migrate to manifest v3 #320

Open
maximbaz opened this issue Mar 28, 2023 · 23 comments
Open

Migrate to manifest v3 #320

maximbaz opened this issue Mar 28, 2023 · 23 comments
Milestone

Comments

@maximbaz
Copy link
Member

This is the tracking item for the manifest migration.

Deadline is June 2023, after which the extension will be set to unlisted in Chrome Web Store.

@maximbaz maximbaz added this to the 3.8.0 milestone Mar 28, 2023
@erayd
Copy link
Collaborator

erayd commented Mar 28, 2023

That June 2023 deadline is now under review, and seems likely to be pushed out. Have been waiting for more info on it since December.

https://developer.chrome.com/docs/extensions/migrating/mv2-sunset/

@erayd
Copy link
Collaborator

erayd commented Mar 28, 2023

Biggest issue currently is premature closedown of service workers, and pending API for intercepting modal auth now that we can no longer use the blocking webrequest API for it.

@maximbaz
Copy link
Member Author

Thanks for the context, very interesting!

@patgmiller
Copy link
Contributor

@maximbaz @erayd What plans / actions are being taken to move forward on this?

@maximbaz
Copy link
Member Author

None on my end, I haven't really looked into it. Thanks for the ping though, the time to do the migration seems to be more and more imminent 🙂

@patgmiller
Copy link
Contributor

Okay, if finding time is this issue I am willing to help out with continuing work or starting on it if need be.

@maximbaz
Copy link
Member Author

Thanks, I suppose a really big part of this would be to figure out what functionality will be broken, and whether there are alternative means of achieving it - so far we have erayd's earlier high-level findings a bit above #320 (comment)

patgmiller pushed a commit to patgmiller/browserpass-extension that referenced this issue Dec 30, 2024
@patgmiller
Copy link
Contributor

patgmiller commented Dec 30, 2024

Thanks, I suppose a really big part of this would be to figure out what functionality will be broken, and whether there are alternative means of achieving it - so far we have erayd's earlier high-level findings a bit above #320 (comment)

@maximbaz @erayd I've been working through this a little bit at a time on this branch https://github.com/patgmiller/browserpass-extension/tree/320-migrate-v3-manifest.

One additional item which wasn't previously mentioned as part of migration from manifest v2 to v3 :

The document object is no longer available to the service worker (background), See https://developer.chrome.com/docs/extensions/reference/api/offscreen#concepts_and_usage

Service workers don't have DOM access

Currently recommended approach is to send clipboard to an "offscreen" document, which I've done in the branch. I haven't done anything with the auth request yet, but i was going to look at next. I also haven't attempted to look at anything with how these might affect Firefox yet.

@maximbaz
Copy link
Member Author

That's an amazing progress, thank you for the update! 🚀

@CarloWood
Copy link

@patgmiller Ah! I found your commit literally 8 minutes after you committed it! And that like 10 minutes after I installed this extension from the official website and noting that it is about to no longer be supported!
I then cloned your repo and tested it: it works. But, I do get this error:
https://gyazo.com/49a79db443a32c46d35d82b38b3e6dce

@CarloWood
Copy link

I think I found the reason here: https://developer.chrome.com/docs/extensions/reference/api/commands#action_commands

This paragraph states that The _execute_action (Manifest V3), _execute_browser_action (Manifest V2), and _execute_page_action (Manifest V2) commands are reserved for the action of trigger your action, browser action, or page action respectively.

In other words, "_execute_browser_action" is Manifest V2.

@CarloWood
Copy link

That was it. I created a pull request. patgmiller#1

@patgmiller
Copy link
Contributor

That's an amazing progress, thank you for the update! 🚀

@maximbaz it looks like Firefox doesn't have plans to deprecate MV2 yet, I'm not sure yet on how if at all FF will support the clipboard approach like chrome of offscreen. It might be easier to try and leave FF as is with MV2 though; do you or @erayd have a preference on trying to migrate FF to MV3 now or later?

@patgmiller
Copy link
Contributor

That was it. I created a pull request. patgmiller#1

@CarloWood it's still very much a WIP, but I will have a look, thanks!

@maximbaz
Copy link
Member Author

Whatever you judge is the most pragmatic choice - it would be nice to not have totally separate codebase for Chrome and FF, and we obviously don't want to block migration if something is not supported on FF. Maybe try MV3 on FF, and whatever functionality if any doesn't work, extract common code in a shared file and call those shared functions in different ways for different browsers?

@patgmiller
Copy link
Contributor

patgmiller commented Dec 30, 2024

Whatever you judge is the most pragmatic choice - it would be nice to not have totally separate codebase for Chrome and FF, and we obviously don't want to block migration if something is not supported on FF. Maybe try MV3 on FF, and whatever functionality if any doesn't work, extract common code in a shared file and call those shared functions in different ways for different browsers?

So far I have FF working on MV3 with the exception of clipboard and i haven't tried web auth in either browser. I'll see how far I can take FF on MV3. It looks like can probably go with your last suggestion baring anything major.

patgmiller pushed a commit to patgmiller/browserpass-extension that referenced this issue Dec 31, 2024
@patgmiller
Copy link
Contributor

patgmiller commented Dec 31, 2024

@maximbaz @erayd copy to clipboard for FF on MV3 works just using the old approach document.execCommand("copy") so it was simple enough to just check if (chrome) { ... use the offscreen, else use the old method.

However clearing the clipboard doesn't work b/c the worker goes idle before the alarm triggers. The alarm is supposed to trigger as soon as it becomes active, but so far that has not been the case.

If I cannot figure out how to get the alarm to trigger on wake, i've thought of two alternatives for which i'd like your input.

  1. Clear the clipboard at runtime.onSuspend might work, but b/c the chrome offscreen approach is async the docs say there's no guarantee it will complete.
  2. Set a one way hash value of the lastCopiedText to local storage for clearClipboard key (same name as alarm) for runtime.onSuspend and check that value on worker wake, then read the clipboard and compare it as before after hashing the read clipboard value.

Thoughts?

@patgmiller
Copy link
Contributor

Another option
3. this stackoverflow suggestion, create activity to keep the server worker alive until the clipboard is cleared https://stackoverflow.com/questions/66618136/persistent-service-worker-in-chrome-extension/

@maximbaz
Copy link
Member Author

maximbaz commented Dec 31, 2024

3 - sounds like a reasonable way to me, in general I think achieving feature parity is the most important part now, and even if we don't like having the tricks of keepign service worker alive, we can find another way later.
2 - I don't quite get the idea yet to be honest, so the clipboard will be cleared on worker wake, but it's not clear to me when worker will wake?

@patgmiller
Copy link
Contributor

i liked 3 best when i found it. and it was pretty simple to implement since we're already using native messaging. added a function

async function keepAlive() {
    chrome.alarms.create("keepAlive", { when: Date.now() + 25e3 });
    await getFullSettings();
}

which is then called in the copy to clipboard, and until either clipboard is cleared or something else external changes the value

// handle fired alarms
chrome.alarms.onAlarm.addListener(async (alarm) => {
    if (alarm.name === "clearClipboard") {
        if ((await readFromClipboard()) === lastCopiedText) {
            copyToClipboard("", false);
        }
        lastCopiedText = null;
    } else if (alarm.name === "keepAlive") {
        const current = await readFromClipboard();
        console.debug("keepAlive fired", { current, lastCopiedText });
        // stop if either value changes
        if (current === lastCopiedText) {
            await keepAlive()
        }
    }
});

@patgmiller
Copy link
Contributor

it worked perfectly, and the service worker is suspended afterwards as it should

patgmiller pushed a commit to patgmiller/browserpass-extension that referenced this issue Dec 31, 2024
patgmiller pushed a commit to patgmiller/browserpass-extension that referenced this issue Dec 31, 2024
@patgmiller
Copy link
Contributor

@maximbaz @erayd looks like the alternative to webRequestBlocking is to replace it with webRequestAuthProvider, and so fare the only issue is that confirm is undefined and causes an error (b/c it's in a background context i think). so we'll probably have to send a message to a content / page doc, i'm thinking just the pop up right now, to provide the "confirm" functionality in a UI.

@maximbaz
Copy link
Member Author

Fine by me, the only downside I can think of is that popup can already be closed by the time we want to confirm, but it's probably an edge case (e.g. after selecting entry, a pinentry is opened, closing the popup).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants