Closed Bug 1009367 Opened 6 years ago Closed 3 years ago

Bookmarks submenus does not respect system theme

Categories

(Firefox :: Theme, defect, P2)

29 Branch
defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: fred.martin-doiva5gn, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: access, regression)

Attachments

(2 files, 1 obsolete file)

Attached image 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.
Component: Untriaged → Theme
Assignee: nobody → ntim007
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #8424296 - Flags: review?(gijskruitbosch+bugs)
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.
Attachment #8424296 - Flags: review?(gijskruitbosch+bugs) → review-
(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.
(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
Flags: needinfo?(gijskruitbosch+bugs)
(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.
Flags: needinfo?(gijskruitbosch+bugs)
(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.
(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.
Attached patch Patch v2Splinter Review
Attachment #8424296 - Attachment is obsolete: true
Attachment #8424320 - Flags: review?(gijskruitbosch+bugs)
Attachment #8424320 - Flags: review?(dao)
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.
Attachment #8424320 - Flags: review?(gijskruitbosch+bugs)
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 on attachment 8424320 [details] [diff] [review]
Patch v2

see comment 10
Attachment #8424320 - Flags: review?(dao) → review-
Flags: firefox-backlog+
Status: ASSIGNED → NEW
Assignee: ntim007 → nobody
Priority: -- → P2
This has been fixed elsewhere.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.