Closed Bug 1501586 Opened 6 years ago Closed 6 years ago

Clean up lwt-*-brighttext attributes properly when switching from a dark theme to a light theme

Categories

(WebExtensions :: Themes, defect)

defect
Not set
normal

Tracking

(firefox65 verified)

VERIFIED FIXED
mozilla65
Tracking Status
firefox65 --- verified

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(1 file)

I've tested only on MacOS so far.

STR:
- Apply the patch from bug 1485599
- Install https://addons.mozilla.org/en-US/firefox/addon/night_owl/ on the build
- Open the bookmarks sidebar
- Click the "owl" toolbar icon from the extension

AR:
- The scrollbar doesn't change colors

ER:
- The scrollbar should have changed colors
Summary: Scrollbars should be re-rendered when their scrollbar-color properties are changed → Scrollbars should be re-rendered when their scrollbar-color property is changed
(oh, I forgot one step in the STR: you need to resize the window so scrollbars appear in the sidebar)
It'd be extra-great if you could attach an HTML-only test-case :)

This is supposed to work at least:

  https://searchfox.org/mozilla-central/rev/a7f4d3ba4fbfe3efbde832869f1d672fce7122f6/layout/style/nsStyleStruct.cpp#4631

Xidorn, any thought?
Flags: needinfo?(xidorn+moz)
I can't seem to make an HTML-only test case, so it's probably a very specific edge case.
I have to sleep now... but some random thought: this may be related to how XUL handles scrollbars which, IIRC, is a different code path and component than what is used in HTML.
This is not a bug of the rendering engine. It is a bug of the frontend code somewhere.

When dark theme is used, "lwt-sidebar-brighttext" attribute is added to the page.sidebar-panel element, and it matches .sidebar-panel[lwt-sidebar-brighttext] which changes the scrollbar-color. When switching back to light theme, this attribute is *not* removed, and thus scrollbar-color is still from .sidebar-panel[lwt-sidebar-brighttext], which is light.

If I close the sidebar in light theme and reopen it, there is no "lwt-sidebar-brighttext" attribute on that element.

So this is neither a style system bug nor a rendering bug. It is most likely a bug in frontend code somewhere which doesn't setup attributes correctly.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(xidorn+moz)
Resolution: --- → INVALID
Thanks for looking into it! Reopening this so I can fix this in the WebExtension side.
Status: RESOLVED → REOPENED
Component: CSS Parsing and Computation → Themes
Product: Core → WebExtensions
Resolution: INVALID → ---
Assignee: nobody → ntim.bugs
Status: REOPENED → ASSIGNED
Summary: Scrollbars should be re-rendered when their scrollbar-color property is changed → Clean up lwt-*-brighttext attributes properly when switching from a dark theme to a light theme
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2b59c32f04d
Clean up lwt-*-brighttext attributes properly when switching from a dark theme to a light theme. r=jaws
https://hg.mozilla.org/mozilla-central/rev/a2b59c32f04d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Verified using the latest Nightly (65.0a1) on Windows 10 x64 and MacOs according to the steps provided in the description, the scrollbar along with all of the sidebar seems to change colors according to the Night Owl scheme, the issue seems to be resolved.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.