migrate to v3 manifest #366

patgmiller · 2025-03-23T03:04:12Z

…vice worker remove mythril from helpers, no access to window update for storage api changes chrome.browserAction => chrome.action.

… fill and copy browserpass#320

… troubleshoot alarm to clear clipboard. browserpass#320

…redential provision not yet implemented. browserpass#320

…just file path. browserpass#320

…serpass#320

…r inject / scripting, browserpass#320

…wserpass#320

patgmiller · 2025-03-23T03:05:16Z

@maximbaz @erayd here's the PR for the v3 manifest

patgmiller · 2025-03-23T03:07:09Z

I've tested in chrome (Version 134.0.6998.119 (Official Build) (arm64)) and firefox (134.0 (aarch64))

maximbaz · 2025-03-23T20:10:47Z

Thanks! Some testing, admittedly without actually looking at the code yet:

I don't experience any popups on gravatar.com logins... I suppose you have fixed it?
Open page that requires basic auth => detached popup opens => close this detached popup without selecting anything => this popup won't ever reappear, it's no longer possible to login to website with basic auth.
When I created a password entry eviltester.com, it shows up in the popup on https://testpages.eviltester.com/styled/auth/basic-auth-test.html before I press the link in the bottom, but once I press the link in the bottom and detached popup opens, the entry is not there (as if it doesn't match the domain). If I create a password entry testpages.eviltester.com instead, then it shows up in both places.

Jassob · 2025-03-24T22:34:24Z

I've installed the extension and will start testing it both in Firefox and Chrome on x86_64.

Is there any specific work flows I should look for? I'm mainly using Ctrl+Shift+L and Ctrl+Shift+F.

maximbaz · 2025-03-24T22:52:38Z

Any bit of testing helps to build confidence, if you spot something weird in your usage, please report back. If you otherwise want to try whatever functionality is easy to test (copy to clipboard, edit passwords, etc), please do it 🙏

@patgmiller I wonder if we should update the part in background.js that handles Ctrl+G? Whatever code is there is probably not working anymore, as it tries to inject basic auth credentials into request?

I'm contemplating whether removing Ctrl+G entirely is a good idea 🤔 If yes now is the best time ever to do it - this shortcut was after all primarily added to enable basic auth flow. But I just don't know if and how many people (ab)use this to use browserpass as a bookmark manager, and whether we should keep it because of that...

patgmiller · 2025-03-25T03:24:11Z

@patgmiller I wonder if we should update the part in background.js that handles Ctrl+G? Whatever code is there is probably not working anymore, as it tries to inject basic auth credentials into request?

I'm contemplating whether removing Ctrl+G entirely is a good idea 🤔 If yes now is the best time ever to do it - this shortcut was after all primarily added to enable basic auth flow. But I just don't know if and how many people (ab)use this to use browserpass as a bookmark manager, and whether we should keep it because of that...

@maximbaz At the very least I would remove the blocking auth part of that shortcut so that it only opens the url in new tab. At that point the updated flow would automatically take over. There's no reason to try and inject anything, I did a lot of experimenting with inject script while trying to make stuff work, later to learn all "error" responses block extension script injection, makes perfect sense. In this case, needs auth is the response to a 400 error (either 401 or 403 depending).

I can't speak to usage case for Ctrl+G though I can see how some people might like to keep it. It's your call really, I'll support whatever you decide.

patgmiller · 2025-03-25T03:28:04Z

Thanks! Some testing, admittedly without actually looking at the code yet:

I don't experience any popups on gravatar.com logins... I suppose you have fixed it?

Yes, sorry I mentioned it, but I also quickly submitted the PR.

Open page that requires basic auth => detached popup opens => close this detached popup without selecting anything => this popup won't ever reappear, it's no longer possible to login to website with basic auth.

I'll look at it, I have an idea what it might be.

When I created a password entry eviltester.com, it shows up in the popup on https://testpages.eviltester.com/styled/auth/basic-auth-test.html before I press the link in the bottom, but once I press the link in the bottom and detached popup opens, the entry is not there (as if it doesn't match the domain). If I create a password entry testpages.eviltester.com instead, then it shows up in both places.

Thanks for the feedback, I'll test it against this example specifically, that's what I need, different approaches to break it 😉

maximbaz · 2025-03-25T10:29:24Z

I can't speak to usage case for Ctrl+G though I can see how some people might like to keep it. It's your call really, I'll support whatever you decide.

OK let's not remove the functionality, but make it a noop, do nothing else except opening a tab. For bonus points, if it's easy and we have infrastructure for this, it would be very nice to show a deprecation warning for this, e.g. something like when people press Ctrl+G or Ctrl+Shift+G, popup says something like "Ctrl+G and Ctrl+Shift+G shortcuts are deprecated and will be removed in future version. You no longer need to open websites that require basic auth using these shortcuts, open websites normally and Browserpass will open a popup for you to choose the credentials.".

At the very least I would remove the blocking auth part of that shortcut so that it only opens the url in new tab.

I know you know, but just to be super explicit here, Ctrl+G opens in the current tab and Ctrl+Shift+G opens in a new tab :)

…RL().host, browserpass#320

patgmiller · 2025-03-26T01:25:53Z

Both of these items are at least be improved, if not fixed. See if you can break them.

Open page that requires basic auth => detached popup opens => close this detached popup without selecting anything => this popup won't ever reappear, it's no longer possible to login to website with basic auth.

@maximbaz this could also happen if someone forgets they already have a window detached popup open, do we need to add a 15 min timeout to close window anyway?

When I created a password entry eviltester.com, it shows up in the popup on https://testpages.eviltester.com/styled/auth/basic-auth-test.html before I press the link in the bottom, but once I press the link in the bottom and detached popup opens, the entry is not there (as if it doesn't match the domain). If I create a password entry testpages.eviltester.com instead, then it shows up in both places.

…pass#320

…ce of url open shortcut, browserpass#320

…agent styles, browserpass#320

patgmiller · 2025-03-29T20:06:50Z

basically give a bit to resolve this because it's causing problems with filling the passwords at the moment.

@maximbaz false alarm ☝🏻 , it was a red herring and not what was causing my issue. I also went back through history. I found that exact log present during all of my videos and even @CarloWood's screenshot of the logs. If I think back over this and my last PR (add / edit) I can recall it being there too now. So I'd like to find it, but don't think it should hold anything up anymore.

I've just about got my work to warn about the non https protocol for auth requests and will push that shortly.

…n user, and proceed only based on confirm, browserpass#320

patgmiller · 2025-03-29T20:29:51Z

@maximbaz so here's what I didi. I've adjusted the modal dialog to also use the style class to have warning / error like the notifications. Here's what it will look like if the user ends up on an auth request with out https

patgmiller · 2025-03-29T20:35:25Z

Currently the only thing I have not done is to increase the height of the window when the modal dialog opens and the window is too small. I will still try and get to it, later today or tomorrow but if not I'll defer to you on if you decide not to hold up the PR for it.

I have tested that the async auth request is still working in both chrome and firefox after my latest changes as well as the other features to fill fields, copy secrets, add and edit, and confirmed that the clipboard is reset after copying password or username.

maximbaz · 2025-03-29T20:41:54Z

That's fantastic!

Just to clarify, I wrote about the deadline mostly to avoid harming your motivation and to give you assurance that I do not intend to repeat our experience with dragging the PRs for too long - for my sake, I'm perfectly happy to wait for your sign-off, it doesn't have to be tomorrow :) If you would like to get that piece done, let's get it in, and don't worry or feel like you have to rush anything - we all, watching this PR and related issue, feel nothing but gratitude for your work, and we will give you whatever time you need.

patgmiller · 2025-03-29T23:26:54Z

ah, got it that makes sense. sounds great! I shouldn't need much longer I feel like it's really close 🤞🏻

…ns, browserpass#320

…ch are clipped, browserpass#320

patgmiller · 2025-03-31T02:53:55Z

modal window content resize is well underway and will also address it for notificaitons #336

Chrome dialog redraw

chrome-modal-window-resize.webm

Firefox dialog redraw

firefox-modal-window-resize.webm

Error notification redraw

chrome-modal-window-resize-on-notification.webm

patgmiller · 2025-03-31T03:18:03Z

@maximbaz okay that should be it. we probably need a final round of testing and you might want to go over my changes since your last review.

Please note this change, I needed to switch back to the user agent approach, since i ran into some errors accessing the url in certain situations.

/**
 * returns true if agent string is Chrome / Chromium
 *
 * @since 3.10.0
 */
function isChrome() {
    const ua = navigator.userAgent;
    const matches = ua.match(/(chrom)/i) || [];
    if (Object.keys(matches).length > 2 && /chrom/i.test(matches[1])) {
        return true;
    }
    return false;
}

maximbaz · 2025-03-31T07:36:12Z

Thanks! Could you please share one or few examples where user agent check is necessary? It would help to know this if some support tickets arise, or if we try to find a third approach later for whatever reason.

patgmiller · 2025-03-31T13:31:15Z

Thanks! Could you please share one or few examples where user agent check is necessary? It would help to know this if some support tickets arise, or if we try to find a third approach later for whatever reason.

This one was the most prominent:

Uncaught (in promise) Error: Cannot access a chrome:// URL background.dist.js:497

This is basically when the user is on the manage chrome extension page, chrome://extensions/?id=..., and attempts to decrypt the secret. In this case what I remembered was the difference between the two approaches is with the user agent approach they could at least decrypt their secret, but with the URL method they could not.

However this morning, when I attempt to duplicate the issue I can decrypt the secret just fine using either approach. So it is possible I was hitting some other use case due to whatever state the changes were in at the time.

I remember wondering if it had to do with chrome-extension:// and host permissions but didn't feel like chrome-extension://*/* was an appropriate host permission request. So I double checked the gobridge host permission manifest and that has the same permissions as ours "host_permissions": ["http://*/*", "https://*/*"],. So I doubt that is the issue.

In the end, because some chrome urls are blocked, I felt that though the url approach more simple, it could potentially be more error prone. But at the moment, I feel like keeping a reference to both until it can be said for sure what issues each might have.

maximbaz · 2025-03-31T13:40:18Z

I see, thanks! I think since browserpass is not expected to work on special pages anyway, maybe it's better to ship with the simpler url based approach, and wait to see whether anyone hits an actual issue 🤔 I just worry about long rabbit hole issues due to people changing their user agents...

…ach temporarily, browserpass#320

…h one to keep or remove, browserpass#320

patgmiller · 2025-03-31T15:50:38Z

Okay, switched it back to simple url approach but left the other commented out for reference with a comment that if issues arise we can revisit/remember the discussion.

maximbaz · 2025-03-31T19:30:08Z

Just to confirm, is there anything else you want to do, or is it ready for merge?

patgmiller · 2025-03-31T21:52:21Z

Just to confirm, is there anything else you want to do, or is it ready for merge?

Nothing else needs to be done, I'd say it's ready to merged.

erayd · 2025-03-31T22:25:28Z

Wow, I have missed a lot here! Just saw a security alert on the Browserpass release accounts, went to check what it was, and found all this. Looks like I have been missing GitHub notifications for this repo for a while; that is now rectified.

Apologies for my absence, and thank you very much indeed @patgmiller and @maximbaz for getting the v3 update over the line!

maximbaz · 2025-03-31T22:31:31Z

Hello 👋 I've just submitted the extensions to Mozilla (has already been approved) and Google.

Even thought it's already merged, if you'd like to do a code review at any point, it will be very appreciated 🙂 We can take all the improvements in the next PR!

Also the README needs to be updated, with all the new permissions and the new way of handling modal HTTP auth (just realized this as I went through the exercise of submitting justifications to Google 😁 )

erayd · 2025-03-31T22:35:55Z

Even thought it's already merged, if you'd like to do a code review at any point, it will be very appreciated 🙂 We can take all the improvements in the next PR!

Can do 🙂

...the new way of handling modal HTTP auth...

I just tested this, and it is NICE! Much more user-friendly than the launch-from-hotkey method we had previously.

erayd · 2025-03-31T22:36:16Z

@maximbaz Are you still on Wire?

erayd

Nice job - thank you, @patgmiller!

src/helpers.js

erayd · 2025-03-31T23:06:55Z

src/background.js

@@ -89,13 +89,91 @@ chrome.commands.onCommand.addListener(async (command) => {
    }
 });

+let currentAuthRequest = null;


This could use a comment explaining why it isn't a race condition (i.e. that the onAuthRequired listener prevents it getting stepped on).

Good question, I wondered near the end of this task if I shouldn't adjust functionality to follow your original approach using something similar to authListeners[tabId];. I will do some additional investigation / research on this and get back to you about it.

Potentially relevant: #374 where multiple popups get shown for the same tab

src/background.js

erayd · 2025-03-31T23:35:33Z

src/background.js

-    document.execCommand("copy");
+async function copyToClipboard(text, clear = true) {
+    if (helpers.isChrome()) {
+        await setupOffscreenDocument("offscreen/offscreen.html");


Does Firefox not support clipboard from offscreen? If it does, IMO it would be sensible to unify this and have all browsers use the same code path for it, rather than making Chrome-specific exceptions.

Firefox doesn't support chrome.offscreen api, see also this #366 (comment)

src/background.js

erayd · 2025-03-31T23:48:14Z

src/background.js

+
+    try {
+        // use for debugging only, since dev tools does not show extension storage
+        await chrome.storage.local.get(console.dir);


Does Firefox work with this now? When originally implemented in Browserpass, this API had problems in Firefox (race conditions IIRC, although it has been a long time since I last looked at it).

If I recall correctly I did this because at the time, when I first started working on the v3 migration, the chrome developer tools didn't show the Extension Storage in Dev Tools.

However, when I run this in Firefox (137.0 (aarch64)) I get the following

Though I'm not sure I follow which part you're asking about. Could you be more specific about which function you're questioning? chrome.storage.local or console.dir?

src/background.js

src/helpers.js

src/offscreen/offscreen.js

patgmiller and others added 16 commits March 22, 2025 20:44

first draft changes required for manifest v3 migration

b634412

minimum required changes to initially get background.js to run as ser…

607da08

…vice worker remove mythril from helpers, no access to window update for storage api changes chrome.browserAction => chrome.action.

browserpass extension first working draft of manifest v3 in chromium:…

f2d7b85

… fill and copy browserpass#320

browserpass copy and fill actions work for FF and Chrome MV3, need to…

41c4a73

… troubleshoot alarm to clear clipboard. browserpass#320

fix clipboard clear and clean up some log messages. browserpass#320

e4fbbd2

add doc string to isChrome method. browserpass#320

70a231f

added async handler in background to webRequest for onAuthRequired, c…

bade912

…redential provision not yet implemented. browserpass#320

for security auth request url should include full extension url, not …

0322cf0

…just file path. browserpass#320

add constant for auth url query param. browserpass#320

c3447f2

async auth modal window now handles filtered search on auth host brow…

7eb6916

…serpass#320

fix display quirk for detached popup window browserpass#320

bc037a7

first working draft of successful auth request browserpass#320

26e5b2d

async auth request is now working, beta stable browserpass#320

6cbb5b3

attempt to fix some issues for fillFields due to difference in api fo…

e9c97d0

…r inject / scripting, browserpass#320

remove extra inject script leftover from initial troubleshooting. bro…

aa3ff8f

…wserpass#320

fixed dispatchFill parsing of results.filledFields, browserpass#320

0da8990

patgmiller added 2 commits March 25, 2025 17:14

add fall back clean up for auth request browserpass#320

b3deb12

fix search for auth to filter on BrowserpassURL().domain instead of U…

448c0c0

…RL().host, browserpass#320

patgmiller added 3 commits March 25, 2025 22:49

extend modal dialog functionality to confirm/acknowledgement, browser…

bb094de

…pass#320

remove obsolete basic auth injection handler and add deprecation noti…

dbb6a5f

…ce of url open shortcut, browserpass#320

add a little style + format to auth deprecation message and override …

fe10fc3

…agent styles, browserpass#320

check for non https protocol when filling fields of auth request, war…

c216788

…n user, and proceed only based on confirm, browserpass#320

patgmiller added 2 commits March 30, 2025 19:14

confirmed must use user agent method for isChrome is certain situatio…

399eccb

…ns, browserpass#320

first version of redraw modal height for dialog and notifications whi…

2b5b83d

…ch are clipped, browserpass#320

patgmiller added 2 commits March 30, 2025 21:01

minor adjustment to refernce diff for notifications, browserpass#320

a93759a

remove console.debug, browserpass#320

a49daae

patgmiller added 2 commits March 31, 2025 09:41

ship with simple url approach, but keep reference to user agent appro…

aadca90

…ach temporarily, browserpass#320

Add note about why keeping commented block and need to determine whic…

6257ec0

…h one to keep or remove, browserpass#320

maximbaz approved these changes Mar 31, 2025

View reviewed changes

maximbaz merged commit ba67b7d into browserpass:master Mar 31, 2025

erayd reviewed Apr 1, 2025

View reviewed changes

patgmiller mentioned this pull request Apr 1, 2025

Migrate to manifest v3 #320

Closed

apiraino mentioned this pull request Apr 3, 2025

Browser window zoom set to 90% hides preferences save button #371

Closed

patgmiller deleted the 320-migrate-v3-manifest branch April 6, 2025 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate to v3 manifest #366

migrate to v3 manifest #366

patgmiller commented Mar 23, 2025 •

edited

Loading

patgmiller commented Mar 23, 2025

patgmiller commented Mar 23, 2025

maximbaz commented Mar 23, 2025 •

edited

Loading

Jassob commented Mar 24, 2025

maximbaz commented Mar 24, 2025 •

edited

Loading

patgmiller commented Mar 25, 2025

patgmiller commented Mar 25, 2025 •

edited

Loading

maximbaz commented Mar 25, 2025

patgmiller commented Mar 26, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 29, 2025

maximbaz commented Mar 29, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 31, 2025 •

edited

Loading

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

erayd commented Mar 31, 2025 •

edited

Loading

maximbaz commented Mar 31, 2025

erayd commented Mar 31, 2025

erayd commented Mar 31, 2025

erayd left a comment

erayd Mar 31, 2025

patgmiller Apr 6, 2025

maximbaz Apr 7, 2025

erayd Mar 31, 2025

patgmiller Apr 7, 2025 •

edited

Loading

erayd Mar 31, 2025

patgmiller Apr 7, 2025

migrate to v3 manifest #366

migrate to v3 manifest #366

Conversation

patgmiller commented Mar 23, 2025 • edited Loading

patgmiller commented Mar 23, 2025

patgmiller commented Mar 23, 2025

maximbaz commented Mar 23, 2025 • edited Loading

Jassob commented Mar 24, 2025

maximbaz commented Mar 24, 2025 • edited Loading

patgmiller commented Mar 25, 2025

patgmiller commented Mar 25, 2025 • edited Loading

maximbaz commented Mar 25, 2025

patgmiller commented Mar 26, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 29, 2025

maximbaz commented Mar 29, 2025

patgmiller commented Mar 29, 2025

patgmiller commented Mar 31, 2025 • edited Loading

Chrome dialog redraw

Firefox dialog redraw

Error notification redraw

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

maximbaz commented Mar 31, 2025

patgmiller commented Mar 31, 2025

erayd commented Mar 31, 2025 • edited Loading

maximbaz commented Mar 31, 2025

erayd commented Mar 31, 2025

erayd commented Mar 31, 2025

erayd left a comment

Choose a reason for hiding this comment

erayd Mar 31, 2025

Choose a reason for hiding this comment

patgmiller Apr 6, 2025

Choose a reason for hiding this comment

maximbaz Apr 7, 2025

Choose a reason for hiding this comment

erayd Mar 31, 2025

Choose a reason for hiding this comment

patgmiller Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

erayd Mar 31, 2025

Choose a reason for hiding this comment

patgmiller Apr 7, 2025

Choose a reason for hiding this comment

patgmiller commented Mar 23, 2025 •

edited

Loading

maximbaz commented Mar 23, 2025 •

edited

Loading

maximbaz commented Mar 24, 2025 •

edited

Loading

patgmiller commented Mar 25, 2025 •

edited

Loading

patgmiller commented Mar 31, 2025 •

edited

Loading

erayd commented Mar 31, 2025 •

edited

Loading

patgmiller Apr 7, 2025 •

edited

Loading