Closed Bug 1176565 Opened 5 years ago Closed 4 years ago

Update tree and list box styling for Windows 10

Categories

(Toolkit :: Themes, defect, P4)

defect

Tracking

()

VERIFIED FIXED
mozilla42
Tracking Status
firefox40 --- verified
firefox41 --- verified
firefox42 --- verified

People

(Reporter: ntim, Assigned: Paenglab)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

Latest leaked build 100147 has a new look for this. The main change is that there is no longer a border around the highlighted items.

Source : https://www.youtube.com/watch?v=tkwkl27eCxw
Attached patch Bug1176565.patch (obsolete) — Splinter Review
This removes the border. Maybe we should wait until a official Win 10 build is out to be sure this comes in.
Attached patch Bug1176565.patch (obsolete) — Splinter Review
Good I awaited the build to test. Where are still borders but fewer.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8629445 - Flags: review?(dao)
Attachment #8625140 - Attachment is obsolete: true
Comment on attachment 8629445 [details] [diff] [review]
Bug1176565.patch

Looks like we should be using CSS variables here. You shouldn't duplicate those massive blocks just to make color adjustments.
Attachment #8629445 - Flags: review?(dao) → review-
Attached patch Patch using variables (obsolete) — Splinter Review
Now with variables.
Attachment #8629445 - Attachment is obsolete: true
Attachment #8629575 - Flags: review?(dao)
Comment on attachment 8629575 [details] [diff] [review]
Patch using variables

> @media (-moz-windows-default-theme) {
>   @media not all and (-moz-os-version: windows-xp) {

So apparently this section is primarily for Windows 8, and then there's a Vista and Win7 section, and another one for Win10. This seems pretty strange. Wouldn't it make more sense to move the Win10 style to the !XP section and the Win8 stuff in a dedicated Win8 section?

>     listitem {
>+      --listitem-selectedBorder: rgb(217,217,217);
>+      --listitem-selectedBottomBorder: rgb(217,217,217);
>+      --listitem-selectedImage: linear-gradient(rgba(190,190,190,.4), rgba(190,190,190,.4));
>+      --listitem-selectedBackground: rgba(190,190,190,.15);
>+      --listitem-selectedFocusBorder: rgb(132,172,221);
>+      --listitem-selectedFocusBottomBorder: rgb(132,172,221);
>+      --listitem-selectedFocusImage: linear-gradient(rgba(131,183,249,.375), rgba(131,183,249,.375));
>+      --listitem-selectedFocusBackground: rgba(131,183,249,.02);
>+      --listitem-selectedCurrentBorder: rgb(125,162,206);
>+      --listitem-selectedFocusCurrentBorder: rgb(132,172,221);
>+      --listitem-selectedFocusCurrentBottomBorder: rgb(132,172,221);
>+      --listitem-selectedFocusCurrentBackground: rgba(131,183,249,.15);
>+    }
>+
>+    listitem {
>       color: -moz-FieldText;

nit: these two rules should be merged. You can add a blank line to separate the variable definitions from the rest.

Would it make sense to add extra variables for colors that are repeated in multiple variables? E.g. rgb(132,172,221) is used a number of times in the above block.
Attachment #8629575 - Flags: review?(dao)
Attached patch Bug1176565.patchSplinter Review
Win10 is now the default and Vista/Win7 and Win8 are in media queries.
Attachment #8629575 - Attachment is obsolete: true
Attachment #8631806 - Flags: review?(dao)
Priority: -- → P4
+ a followup to update the twisty/expander icon for containers in the treeview?
...that would be bug 1181548, I think.
Blocks: theme-win10
No longer blocks: windows-10
Attachment #8631806 - Flags: review?(dao) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8ca313c345e1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8631806 [details] [diff] [review]
Bug1176565.patch

Approval Request Comment
[Feature/regressing bug #]: Windows 10 UI theme adjustments
[User impact if declined]: Firefox on Windows 10 won't match native/desired look-and-feel
[Describe test coverage new/current, TreeHerder]: local testing, landed on m-c
[Risks and why]: low risk, css only
[String/UUID change made/needed]: none
Attachment #8631806 - Flags: approval-mozilla-beta?
Attachment #8631806 - Flags: approval-mozilla-aurora?
Comment on attachment 8631806 [details] [diff] [review]
Bug1176565.patch

CSS only change, updates to match win10 theme UI changes.
Attachment #8631806 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Comment on attachment 8631806 [details] [diff] [review]
Bug1176565.patch

Style change for Windows 10. Beta+
Attachment #8631806 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced the bug with Nightly 41.0a1 (2015-06-21) by highlighting items from the History/Bookmarks Sidebar and from the Library.

This is verified fixed on Beta 40.0b8 (20150727174134) and Nightly 42.0a1 (2015-07-27), using Windows 10 Pro x64 (Build 10240). Richard, apart from the Sidebars and Library, are there any other places I should be looking at for this fix?
Status: RESOLVED → VERIFIED
Flags: qe-verify+ → needinfo?(richard.marti)
I don't know of other laces in FX.
Flags: needinfo?(richard.marti)
(In reply to Andrei Vaida, QA [:avaida] from comment #16)
> Reproduced the bug with Nightly 41.0a1 (2015-06-21) by highlighting items
> from the History/Bookmarks Sidebar and from the Library.
> 
> This is verified fixed on Beta 40.0b8 (20150727174134) and Nightly 42.0a1
> (2015-07-27), using Windows 10 Pro x64 (Build 10240). Richard, apart from
> the Sidebars and Library, are there any other places I should be looking at
> for this fix?
The <listbox> style should also be checked. Here's an url using those : chrome://browser/content/preferences/languages.xul
Thanks, Tim! I've verified the <listbox> style as well across branches, Aurora 41.0a2 (2015-07-31) included.
You need to log in before you can comment on or make changes to this bug.