1009367 - Bookmarks submenus does not respect system theme

Status: RESOLVED WORKSFORME

(Firefox :: Theme, defect, P2)

Description

[fred.martin-doiva5gn]

Attachment: High Contrast #1

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140506152807

Steps to reproduce:

Use the Windows 7 high contrast theme "High Contrast #1".
Start Firefox 29.0.1.
Open bookmarks.
Navigate to a submenu.


Actual results:

The bookmark submenus does not respect the system's colour settings.


Expected results:

The bookmark submenus should respect the system's colour settings. Like the bookmarks menus does.

Comment 1

[Tim Nguyen :ntim]

Attachment: Patch v1

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8424296 [details] [diff] [review]
Patch v1

-moz-fieldText needs to be used as the text color along with the -moz-field background.

Comment 3

[:Gijs (out for now; he/him)]

(In reply to Dão Gottwald [:dao] from comment #2)
> Comment on attachment 8424296 [details] [diff] [review]
> Patch v1
> 
> -moz-fieldText needs to be used as the text color along with the -moz-field
> background.

This will need to be fixed on Linux as well. AFAICT it's not necessary to do anything on OS X.

Comment 4

[Tim Nguyen :ntim]

(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Dão Gottwald [:dao] from comment #2)
> > Comment on attachment 8424296 [details] [diff] [review]
> > Patch v1
> > 
> > -moz-fieldText needs to be used as the text color along with the -moz-field
> > background.
> 
> This will need to be fixed on Linux as well. AFAICT it's not necessary to do
> anything on OS X.

I couldn't find any bookmarks submenu code on linux

Comment 5

[:Gijs (out for now; he/him)]

(In reply to Tim Nguyen [:ntim] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > (In reply to Dão Gottwald [:dao] from comment #2)
> > > Comment on attachment 8424296 [details] [diff] [review]
> > > Patch v1
> > > 
> > > -moz-fieldText needs to be used as the text color along with the -moz-field
> > > background.
> > 
> > This will need to be fixed on Linux as well. AFAICT it's not necessary to do
> > anything on OS X.
> 
> I couldn't find any bookmarks submenu code on linux

There is some in linux/customizableui/panelUIOverlay.css.

It seems like the toplevel bookmarks menu is using -moz-dialog/-moz-dialogtext on Linux, and the submenus are using -moz-appearance: menu/menutext. On the GTK theme that happens to be configured for me right now, that means the main menu is grey and the submenus are white. They should be consistent.

Comment 6

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #5)
> It seems like the toplevel bookmarks menu is using
> -moz-dialog/-moz-dialogtext on Linux, and the submenus are using
> -moz-appearance: menu/menutext. On the GTK theme that happens to be
> configured for me right now, that means the main menu is grey and the
> submenus are white. They should be consistent.

Which would be a separate bug.

Comment 7

[:Gijs (out for now; he/him)]

(In reply to Dão Gottwald [:dao] from comment #6)
> (In reply to :Gijs Kruitbosch from comment #5)
> > It seems like the toplevel bookmarks menu is using
> > -moz-dialog/-moz-dialogtext on Linux, and the submenus are using
> > -moz-appearance: menu/menutext. On the GTK theme that happens to be
> > configured for me right now, that means the main menu is grey and the
> > submenus are white. They should be consistent.
> 
> Which would be a separate bug.

*shrug*. I think it's the same problem, but if you prefer to followup this, feel free.

Comment 8

[Tim Nguyen :ntim]

Attachment: Patch v2

Comment 9

[:Gijs (out for now; he/him)]

Comment on attachment 8424320 [details] [diff] [review]
Patch v2

Review of attachment 8424320 [details] [diff] [review]:
-----------------------------------------------------------------

I'm uncomfortable with the opacity solution; the contrast might not be good enough. I would have thought there was a constant we can use in that case. Leaving review for Dão, who might know whether there is.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8424320 [details] [diff] [review]
Patch v2

> #BMB_bookmarksPopup .menu-text {
>-  color: #000;
>+  color: -moz-fieldtext;
> }

Does this rule even work? If it did, how did we arrive at attachment 8421479 [details]?

> #BMB_bookmarksPopup .subviewbutton[disabled=true] > .menu-text {
>-  color: #6d6d6d;
>+  opacity: 0.6;
> }

You can use GrayText here.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8424320 [details] [diff] [review]
Patch v2

see comment 10

Comment 12

[Dão Gottwald [:dao]]

This has been fixed elsewhere.