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

Bad interaction with usercss extensions in Firefox #224

Open
elprans opened this issue Aug 2, 2020 · 6 comments · May be fixed by #230
Open

Bad interaction with usercss extensions in Firefox #224

elprans opened this issue Aug 2, 2020 · 6 comments · May be fixed by #230

Comments

@elprans
Copy link

elprans commented Aug 2, 2020

General information

  • Operating system + version: Gentoo Linux
  • Browser + version: Firefox 79
  • Information about the host app:
    • How did you install it? -- emerge browserpass
  • Information about the browser extension:
    • How did you install it? From addons.mozilla.com.
    • Browserpass extension version as reported by your browser: 3.5.0

Exact steps to reproduce the problem

  1. Install any style-customization extension (e.g. stylus, github-dark etc.)

  2. Go to the styled site. When browserpass extension is installed a brief flash of originally-styled content is shown.

This started happening in FF 79 for me. Can be reproduced in the official beta for FF 80 on a fresh profile with only stylus and browserpass installed.

@erayd
Copy link
Collaborator

erayd commented Aug 2, 2020

We don't remove user styles, or reset styles on the page - in fact, we don't touch the original page's CSS at all! If the original content is being shown, this seems more likely to be a problem with Firefox, or with the user style extension being used, rather than a Browserpass issue.

When browserpass extension is installed a brief flash of originally-styled content is shown.

If this only occurs when the extension is installed, this isn't something that matters. Is that what you meant though? It seems odd to log a bug for a brief style flash that occurs only during installation, which is something most users only ever do once - so to clarify, can you explain what you mean by "installed"?

@elprans
Copy link
Author

elprans commented Aug 2, 2020

so to clarify, can you explain what you mean by "installed"?

I didn't mean "during installation", I meant that the flash only occurs if the browserpass extension is installed.

FWIW, I've been able to make it go away by commenting out updateMatchingPasswordsCount(tabId); from the tab event listener. I suspect that the event handlers are probably handled sequentially, so browserpass effectively blocks stylus long enough for the unstyled flash to be rendered.

@erayd
Copy link
Collaborator

erayd commented Aug 2, 2020

I didn't mean "during installation", I meant that the flash only occurs if the browserpass extension is installed.

OK, that makes sense - misunderstood you sorry.

FWIW, I've been able to make it go away by commenting out updateMatchingPasswordsCount(tabId); from the tab event listener. I suspect that the event handlers are probably handled sequentially, so browserpass effectively blocks stylus long enough for the unstyled flash to be rendered.

That is really useful info, thank you. I will see if I can reproduce that here, and if so whether shifting that code out of the main execution path of the event handler is sufficient to deal with it.

@erayd
Copy link
Collaborator

erayd commented Aug 2, 2020

OK, so I was unable to reproduce this under normal conditions, but I was able to reproduce it when running a significant CPU hog at the same time (stress -c 16 on an i5-4258U), plus Chrome playing an embedded YouTube video in the background. I tested using Stylus as the userstyle extension. I tested using a blank page with one line of text on an HTTPS origin. The user style simply changed the page background colour.

There seems to be something a bit weird about Firefox's event handler implementation:

  1. It doesn't happen every time, but seems to be sporadic.
  2. No apparent difference between loading the site in a new tab, or refreshing an existing one.
  3. The more tabs I have open, the longer the average duration of the unstyled flash, but some flashes are still very quick.
  4. The more tabs I have open, from any origin (styled or not), the more likely the issue is to occur.
  5. If I refresh them, tabs opened earlier don't seem to show any different behavior to tabs opened later.
  6. There doesn't seem to be any particular pattern as to when it does or does not occur, other than being more likely under high CPU load.
  7. Replacing the updateMatchingPasswordsCount(tabId) call with console.log("something") resolves the issue.
  8. Wrapping the call with a 5 second setTimeout does not resolve it, even though the timeout does not fire until after the issue has both occurred and resolved, and after the page has finished rendering and is displayed as it ought to according to the user style.

That last point (8) is the strangest part of it all - there should be no additional load of any note caused by this handler until after the timeout fires, and yet the mere presence of the call is enough to trigger it.

@maximbaz - do you have any particular thoughts on this one? I'm confident that this is a Firefox problem, rather than some sort of bug in Browserpass. I cannot currently think of a reasonable strategy to mitigate it without removing the feature entirely.

@maximbaz
Copy link
Member

maximbaz commented Aug 3, 2020

Interesting investigation, and point 8 is indeed the strangest of them all!

I cannot think of anything else we can do, I dont think we should remove the functionality completely but we do have #103 to optionally disable the badge, shall we merge this into #103?

@elprans
Copy link
Author

elprans commented Aug 3, 2020

An option to turn off the badge icon seems like a good workaround.

@maximbaz maximbaz linked a pull request Sep 9, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants