Use the non-native theme on Windows.
Categories
(Core :: Widget: Win32, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox141 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(1 file)
| Assignee | ||
Comment 1•7 months ago
|
||
This is to be landed after the soft-freeze of course. This patch aligns
with Linux, removing the ability to draw native-looking buttons and
progress-bars on Windows.
Note that checkboxes, menus, and a bunch of other things already use
non-native drawing (including virtually all our main browser UI), so
this only affects "secondary" UI: the old profile manager, a couple
dialogs, the bookmarks sidebar panel searchbox, page info window...
Also, we already effectively fallback to the non-native theme in dark
mode, and our non-native theme looks reasonably windows-like.
Advantages (apart from not having to maintain all this code) is that the
non-native theme is also faster, and this paves the way towards making
the layout side of theming simpler.
In order to make buttons still react to hover feedback, fix the
button{hover/active} system colors to match the windows task manager.
Things that I've tested:
- Buttons (profile manager, create profile wizard, page info window).
- Trees and other listboxes (profile manager, page info window).
- <input type=text> (bookmarks sidebar)
- <menulist> (ask to save dialog, note it needs D250006 to show the
dropmarker and have the right spacing).
Things that were supported but are unused (to the best of my ability):
- <progress> (there are a few like the downloads popup but they're
custom). - <input type=range>
Note that the places window already uses non-native painting.
IMHO the only thing that could be considered a regression is the
button rendering (debatable tho). I think longer term we might want to
move to our design system's button tokens and such, but over all things
look a bit more consistent and I'm happy to look at improving the colors
here in a follow-up to match the Win32 blue or something else if we deem
it worth it.
Comment 2•7 months ago
•
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)
- <progress> (there are a few like the downloads popup but they're
custom).
Dao asked me an opinion related to downloads.
I agree the downloads progress is probably the most prominent one, there's various other consumers, iirc master password uses it for password strength, Firefox Refresh, Add-on install... Maybe add-ons is the only other case that is more commonly seen.
Downloads use their custom styling from quite some time, and honestly we didn't see any major user concerns. I guess the hypothesis here is that native styling of progress bars is not something users perceive as an important benefit, and as far as our styling is not too far away from the native one, it will be ok.
I think I concur with that, if the benefit is tangible there's no strong reason to keep it. But that's in the end a decision of the Theme owners, as it's more related to the direction of the theme (how close to native it should be and at which cost).
| Assignee | ||
Comment 3•7 months ago
|
||
(In reply to Marco Bonardo [:mak] from comment #2)
I agree the downloads progress is probably the most prominent one, there's various other consumers
iirc master password uses it for password strength
That already uses the non-native theme, so no change here.
Downloads use their custom styling from quite some time, and honestly we didn't see any major user concerns. I guess the hypothesis here is that native styling of progress bars is not something users perceive as an important benefit, and as far as our styling is not too far away from the native one, it will be ok.
I think I concur with that, if the benefit is tangible there's no strong reason to keep it. But that's in the end a decision of the Theme owners, as it's more related to the direction of the theme (how close to native it should be and at which cost).
To be clear, what we're removing is "Windows-native" progressbars. You still get the "Firefox themed" progressbars that content gets. IMHO the Firefox progress bar looks more modern than the current Windows native one (which is just win32-style progressbar), and yeah the cost of maintaining that code isn't worth it, I think. It also triggers slower painting code-paths and so on.
Comment 6•7 months ago
|
||
Revert for causing bc failures on browser_1003588_no_specials_in_panel.js.
[task 2025-05-29T01:49:45.326Z] 01:49:45 INFO - TEST-PASS | browser/components/customizableui/test/browser_1003588_no_specials_in_panel.js | Area widget-overflow-fixed-list should have 0 items. -
[task 2025-05-29T01:49:45.327Z] 01:49:45 INFO - Buffered messages finished
[task 2025-05-29T01:49:45.328Z] 01:49:45 INFO - TEST-UNEXPECTED-FAIL | browser/components/customizableui/test/browser_1003588_no_specials_in_panel.js | Uncaught exception in test bound checkDragging - at chrome://mochitests/content/browser/browser/components/customizableui/test/head.js:221 - TypeError: can't access property "parentNode", aToDrag is null
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - Stack trace:
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - simulateItemDrag@chrome://mochitests/content/browser/browser/components/customizableui/test/head.js:221:5
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - checkDragging@chrome://mochitests/content/browser/browser/components/customizableui/test/browser_1003588_no_specials_in_panel.js:111:21
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - async*handleTask@chrome://mochikit/content/browser-test.js:1170:26
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1242:18
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - async*Tester_execTest@chrome://mochikit/content/browser-test.js:1383:14
[task 2025-05-29T01:49:45.329Z] 01:49:45 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1159:14
[task 2025-05-29T01:49:45.330Z] 01:49:45 INFO - SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13
[task 2025-05-29T01:49:45.330Z] 01:49:45 INFO - Leaving test bound checkDragging
[task 2025-05-29T01:49:45.331Z] 01:49:45 INFO - Entering test bound asyncCleanup
[task 2025-05-29T01:49:45.332Z] 01:49:45 INFO - GECKO(6028) | [Child 748, Main Thread] WARNING: 'ps->NeedLayoutFlush()', file /builds/worker/checkouts/gecko/dom/base/Document.cpp:18377
[task 2025-05-29T01:49:45.770Z] 01:49:45 INFO - Leaving test bound asyncCleanup
[task 2025-05-29T01:49:45.966Z] 01:49:45 INFO - GECKO(6028) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to C:\Users\task_174847940919596\AppData\Local\Temp\tmpnfa8evj8.mozrunner\runtests_leaks_tab_pid8492.log
[task 2025-05-29T01:49:46.282Z] 01:49:46 INFO - GECKO(6028) | MEMORY STAT | vsize 2112519MB | vsizeMaxContiguous 67309218MB | residentFast 389MB | heapAllocated 161MB
[task 2025-05-29T01:49:46.284Z] 01:49:46 INFO - TEST-OK | browser/components/customizableui/test/browser_1003588_no_specials_in_panel.js | took 2065ms
[task 2025-05-29T01:49:46.299Z] 01:49:46 INFO - GECKO(6028) | [Child 9180: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 124edfd6000 == 2 [pid = 9180] [id = 1]
[task 2025-05-29T01:49:46.300Z] 01:49:46 INFO - GECKO(6028) | [Child 9180: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 5 (124eb981000) [pid = 9180] [serial = 5] [outer = 0]
[task 2025-05-29T01:49:46.301Z] 01:49:46 INFO - GECKO(6028) | [Child 9180: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 6 (124edf32600) [pid = 9180] [serial = 6] [outer = 124eb981000]
[task 2025-05-29T01:49:46.334Z] 01:49:46 INFO - checking window state
[task 2025-05-29T01:49:46.383Z] 01:49:46 INFO - TEST-START | browser/components/customizableui/test/browser_1008559_anchor_undo_restore.js
| Assignee | ||
Updated•7 months ago
|
Comment 9•7 months ago
|
||
Emilio, is that a change that will result into an end-user visible change (enhancement) that should be mentioned in our release notes? Thanks
| Assignee | ||
Comment 10•7 months ago
|
||
Not particularly. Is a change to how our buttons look in legacy UI, and they now also render a bit faster. Long-term it unblocks enhancements tho.
Updated•6 months ago
|
Updated•6 months ago
|
Description
•