Update tree and list box styling for Windows 10

VERIFIED FIXED in Firefox 40

Status

()

P4
normal
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: ntim, Assigned: Paenglab)

Tracking

(Blocks: 1 bug)

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox40 verified, firefox41 verified, firefox42 verified)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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
(Assignee)

Comment 1

4 years ago
Created attachment 8625140 [details] [diff] [review]
Bug1176565.patch

This removes the border. Maybe we should wait until a official Win 10 build is out to be sure this comes in.
(Assignee)

Comment 2

4 years ago
Created attachment 8629445 [details] [diff] [review]
Bug1176565.patch

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)
(Assignee)

Updated

4 years ago
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-
(Assignee)

Comment 4

4 years ago
Created attachment 8629575 [details] [diff] [review]
Patch using variables

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)
(Assignee)

Comment 6

4 years ago
Created attachment 8631806 [details] [diff] [review]
Bug1176565.patch

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: 1158143
No longer blocks: 1077146
Attachment #8631806 - Flags: review?(dao) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8ca313c345e1
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
(Assignee)

Comment 11

3 years ago
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?
status-firefox40: --- → affected
status-firefox41: --- → affected
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
status-firefox40: fixed → verified
status-firefox42: fixed → verified
Flags: qe-verify+ → needinfo?(richard.marti)
(Assignee)

Comment 17

3 years ago
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.
status-firefox41: fixed → verified
You need to log in before you can comment on or make changes to this bug.