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

Firefox default theme changes with UBO (non-webext) v1.11.5rc3+ #2502

Closed
Vangelis66 opened this issue Apr 1, 2017 · 23 comments
Closed

Firefox default theme changes with UBO (non-webext) v1.11.5rc3+ #2502

Vangelis66 opened this issue Apr 1, 2017 · 23 comments

Comments

@Vangelis66
Copy link

OS:
Windows Vista SP2 32bit fully updated
Aero Theme Enabled

Firefox version:
52.0.2esr (Vista users moved to ESR channel)

UBO version:
1.11.5rc5 (installed from Github, AMO is on 1.11.5rc4 currently)
DEFAULT settings, all filters updated manually.

Any version of UBO after 1.11.5rc2 alters default colours and other aspects
of Fx default theme after browser is restarted - the issue manifested itself
today, when I manually updated (from AMO) v1.11.5rc1 to v1.11.5rc3;
all versions after that continue to manifest the bug...

Screenshot from Fx's Safe Mode:

safemode

Toolbar background light blue, sidebar background paler blue,
"bookmarks" header in bold and "unified" with the rest of the sidebar.

Screenshot with only latest UBO 1.11.5rc5 enabled:

ubo_1 11 5rc3

Toolbar background light grey/white, sidebar background white, "bookmarks"
header not in bold, in its own section above the rest of the sidebar,
with a pale blue background...

Please advise how to rectify things...
Thanks for your efforts in maintaining UBO!

@SW1FT
Copy link
Contributor

SW1FT commented Apr 1, 2017

I just noticed this on Windows 10. uBO needs to be enabled at browser start, if it's disabled and then you enable it, it won't change the theme.

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

Any errors at the browser console?

The only meaningful change I can think which could be related is the fix to #2493.

I do not have Windows by the way, I won't be able to even try to reproduce this.

@SW1FT
Copy link
Contributor

SW1FT commented Apr 1, 2017

Unfortunately, I don't see any errors in the browser console.

@Vangelis66
Copy link
Author

uBO needs to be enabled at browser start,
if it's disabled and then you enable it,
it won't change the theme.

True, but not practical;

  1. Start Fx with UBO disabled
  2. Enable UBO through addons manager.
  3. Remember to disable UBO before exiting the browser,
    else, after a browser relaunch, browser GUI is again altered!

Any errors at the browser console?

I am not an experienced user of UBO, so unsure what errors to
look for; Fx 52.0.2esr-win32 with only about:addons open:

Content Security Policy: Couldn’t process unknown directive ‘worker-src’  (άγνωστο)
Content Security Policy: Directive ‘frame-src’ has been deprecated. Please use directive ‘child-src’ instead.  (άγνωστο)
Self Repair active  selfrepair.e769dcc4d6f0dedd8f82.js:9:8049
Property contained reference to invalid variable.  Error in parsing value for ‘background-color’.  Καταφυγή σε ‘initial’.  webconsole.css:406:9402
Property contained reference to invalid variable.  Error in parsing value for ‘border-top-color’.  Καταφυγή σε ‘initial’.  webconsole.css:407:9459
Property contained reference to invalid variable.  Error in parsing value for ‘border-top-style’.  Καταφυγή σε ‘initial’.  webconsole.css:407:9459
Property contained reference to invalid variable.  Error in parsing value for ‘border-top-width’.  Καταφυγή σε ‘initial’.  webconsole.css:407:9459
Property contained reference to invalid variable.  Error in parsing value for ‘background-image’.  Καταφυγή σε ‘initial’.  webconsole.css:444:10354
Property contained reference to invalid variable.  Error in parsing value for ‘border-bottom-width’.  Καταφυγή σε ‘initial’.  widgets.css:85:2908
Property contained reference to invalid variable.  Error in parsing value for ‘border-top-width’.  Καταφυγή σε ‘initial’.  widgets.css:84:2850
Property contained reference to invalid variable.  Error in parsing value for ‘min-height’.  Καταφυγή σε ‘initial’.  widgets.css:82:2742
Property contained reference to invalid variable.  Error in parsing value for ‘margin-bottom’.  Καταφυγή σε ‘initial’.  widgets.css:87:3041
Property contained reference to invalid variable.  Error in parsing value for ‘margin-top’.  Καταφυγή σε ‘initial’.  widgets.css:86:2972

The only meaningful change I can think which could be related is the fix to #2493.

The culprit commit is somewhere between
1.11.5rc2...1.11.5rc3

I do not have Windows by the way, I won't be able to even try to reproduce this.

... Most unfortunate, I guess...
Have downgraded to 1.11.5rc2 for now, until this (Windows only?) issue is sorted out...
Regards.
[BTW, I'll be unavailable for at least the next 3 hours...]

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

Is this a Firefox theme? I understood the issue to be with a OS theme. If this is a Firefox theme, I would like to have a link to it.

@Vangelis66
Copy link
Author

Vangelis66 commented Apr 1, 2017

I understood the issue to be with a OS theme

Not at all!

If this is a Firefox theme

Default Fx theme, i.e. no persona (lightweight theme) or third party (full) theme installed.

I would like to have a link to it.

Firefox 52.0.2 x86 release channel (en-US locale):

https://ftp.mozilla.org/pub/firefox/releases/52.0.2/win32/en-US/

Firefox 52.0.2 x86 ESR channel (en-US locale):

https://ftp.mozilla.org/pub/firefox/releases/52.0.2esr/win32/en-US/

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

Default Fx theme

What is "Aero Theme" then? On my side there is only one theme available, "Default 52.0.2".

@Vangelis66
Copy link
Author

What is "Aero Theme" then?

Windows "Aero Theme" that can be enabled on Windows Vista and Windows 7,
but removed in Windows 8, 8.1, 10 by Microsoft:

https://en.wikipedia.org/wiki/Windows_Aero

I just mentioned I have it enabled in my initial post,
under OS specifics, for completeness reasons.

The issue reported has nothing to do with the OS theme,
as it happens with Windows Basic theme also, plus @SW1FT
already said that has the bug on Windows 10!

Thank you...

@SW1FT
Copy link
Contributor

SW1FT commented Apr 1, 2017

@gorhill Also found another issue: I can't get the logger to open through uBO's panel.

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

I can't get the logger to open through uBO's panel.

I can reproduce. It seems the issue is only with the logger as a detached window. When using shift-click to go back to non-detached, the logger appears as a tab.

@SXZ1
Copy link

SXZ1 commented Apr 1, 2017

Same problem. Nightly 01-04-17, uBlock Origin 1.11.5rc4, Windows 8.1 with latest updates. Hamburger menu, close confirmation dialog and Classic Theme Restorer menus are also affected, Virtual_ManPL posted more comparison screenshots in this post:
http://forums.mozillazine.org/viewtopic.php?p=14740758#p14740758

@zeromnsch
Copy link

The same problems appear when updating umatrix from version 0.9.9b7 to 0.9.9b8. I use ublock and umatrix. With combination 0.9.9b7 and 1.11.5rc2 everything is still okay. Update one of the two and the problems appear. Color representation and window elements are disturbed.

@gorhill gorhill closed this as completed in 9f4a879 Apr 1, 2017
@gorhill gorhill reopened this Apr 1, 2017
@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

The same problems appear when updating umatrix from version 0.9.9b7 to 0.9.9b8.

Little doubt this is caused by dc06d5f. Maybe @gijsk can provide some insights, because frankly I have no idea where to start to look.

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

I can't get the logger to open through uBO's panel.

Fixed with 9f4a879.

@spectatorBH
Copy link

Seems like the current fix to #2493 has even more nasty side-effect on PaleMoon: the uBO icon simply disappears from the browser's toolbar after updating to 1.11.5rc3+ and can't be found in customize window either.

Relevant browser console errors after disabling/re-enabling uBO 1.11.5rc3+:

[22:48:37.995] WARN addons.xpi: Add-on uBlock0@raymondhill.net is missing bootstrap method shutdown @ resource://gre/modules/XPIProvider.jsm:3927

[22:48:40.395] WARN addons.xpi: Error loading bootstrap.js for uBlock0@raymondhill.net: SyntaxError: missing : after property id @ resource://gre/modules/XPIProvider.jsm -> jar:file:///C:/Users//AppData/Roaming/Moonchild%20Productions/Pale%20Moon/Profiles/XXXXX.default/extensions/uBlock0@raymondhill.net.xpi!/bootstrap.js:100

[22:48:40.395] WARN addons.xpi: Add-on uBlock0@raymondhill.net is missing bootstrap method startup @ resource://gre/modules/XPIProvider.jsm:3927

Noticed the issue today on PaleMoon 26.5.0 (x64) + uBO 1.11.5rc3/4/5 under Windows 7 x64 (fully patched) after manually updating via GitHub. uBO 1.11.5.rc2 and earlier worked fine in this regard.

Should I file a separate bug report on this?

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

Works fine on Pale Moon 27.2.1. However from the error you report, I can probably fix this -- I think there is some ES6-only syntax which has been introduced in there ("method definition shorthand syntax").

@gijsk
Copy link
Contributor

gijsk commented Apr 1, 2017

I can reproduce the theme breakage on my Windows machine. The same thing happens on Nightly and 53. I have no idea why right now, there aren't obvious errors indicating something went wrong, and there's no obvious connection between the changes the patch makes and theming (or even the window in which things break, as the changes all relate to the 'hidden' window and/or offscreen drawing). We use the same APIs without issues from various other bits of Firefox already.

I tested the original patch I provided on OS X, which would explain why I didn't notice it before.

@gijsk
Copy link
Contributor

gijsk commented Apr 1, 2017

Update: this goes away (on my machine) if we wait 5 seconds to actually load a page into the windowless browser. Of course, that's not an actual fix, the question is what's finishing in those 5 seconds that is making this work instead of break...

@gorhill
Copy link
Owner

gorhill commented Apr 1, 2017

Isn't possible with the browser toolbox's inspector to find out why the theme's styles are not being applied?

@gijsk
Copy link
Contributor

gijsk commented Apr 1, 2017

That's not a bad idea, though it's still not immediately obvious what's wrong.

As far as I can tell Gecko's windows widget code gets... confused... for some reason, about the state of the world. Specifically, the media query (-moz-windows-default-theme) (which is supposed to match if using aero on vista/win7, or on all non-high-contrast windows 8/10 themes) doesn't match. This means we basically style Firefox as if you're using a high contrast theme, and use many more "native" colours rather than our own palette (for reasons not really relevant here, Windows exposes native colours that don't actually match what you'd be used to seeing on XP and later for non-classic themes, like Aero and all non-high-contrast themes on Win8 and Win10, which is why it looks so spectacularly ugly rather than like we actually tried and failed to apply Windows native styling). All the CSS and JS is loaded correctly, which is why there's no obvious errors in the console. But the media queries being wrong explain the broken appearance. There's probably other Windows-related media queries that are doing the opposite of what they should be here.

What I suspect is that the Windows widget code is caching some data about the state of the world, and by creating a window earlier than we used to, that code is getting confused. By pure coincidence, this seems like it might be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1351676 which I happened to file earlier this week...

In any case, what I imagine should fix things (esp. on short notice here) is to simply wait until the hidden window is loaded, like before, but then still end up using a windowless browser. I'll put up a PR once I've made that code not-completely-utterly hacky (within the next half hour, I expect).

My apologies for the mess! (and, to be clear, I normally work on frontend code, like js/css/xul, and so the actual widget code here would take me too long to figure out for a Saturday night...)

gijsk added a commit to gijsk/uBlock that referenced this issue Apr 1, 2017
@gijsk
Copy link
Contributor

gijsk commented Apr 1, 2017

OK, I put up a PR. @gorhill, can you check it looks OK to you?

This fixes things on my Windows machine, and logically speaking it makes some sense (though only some...) that it does. I'll try to see if I can find someone to look into the breakage this caused on Monday (on the Gecko side). Gecko shouldn't break like this just because consumers happen to create stuff early (or we should just be invoking the bootstrap methods later). That said, if the uBO codebase needs to stay compatible with older versions of Gecko (like Pale Moon) I'm not sure at what point this hack could be removed...

@gorhill gorhill closed this as completed in 53a794d Apr 1, 2017
@Vangelis66
Copy link
Author

... I downloaded 1.11.5rc5 from GitHub (file uBlock0.firefox.xpi) and extracted locally;
then downloaded patched file by @gijsk from:

https://github.com/gijsk/uBlock/raw/bfabe3d7bd9e3a5587aea6d96dc8c3014583eb12/platform/firefox/bootstrap.js

and this was used to overwrite existing version inside 1.11.5rc5; then created a new (unsigned)
xpi and installed xpi from file.
I can confirm original issue is no more observed:

fix

Thanks all :-)

gorhill added a commit to gorhill/uMatrix that referenced this issue Apr 1, 2017
@gijsk
Copy link
Contributor

gijsk commented Apr 3, 2017

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1353022 on the gecko widget issue.

Noxgrim pushed a commit to Noxgrim/uMatrix that referenced this issue Dec 29, 2021
Noxgrim pushed a commit to Noxgrim/uMatrix that referenced this issue Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants