Closed Bug 1042268 Opened 5 years ago Closed 5 years ago

"Show All Bookmarks" item of Bookmarks widget dropdown is almost invisible in High contrast themes

Categories

(Firefox :: Theme, defect)

x86_64
Windows 7
defect
Not set
Points:
3

Tracking

()

VERIFIED FIXED
Firefox 34
Iteration:
34.3
Tracking Status
firefox31 --- wontfix
firefox32 - affected
firefox33 - affected
firefox34 --- verified
firefox-esr24 --- unaffected
firefox-esr31 - affected

People

(Reporter: alice0775, Assigned: alexbardas, Mentored)

References

Details

(Keywords: access, regression, Whiteboard: [lang=css][good first bug])

Attachments

(3 files, 3 obsolete files)

Attached image screenshot
Steps To Reproduce:
1. Set High contrast themes in windows7 visual style
2. Open Firefox
3. Open Bookmarks widget dropdown

Actual Results:
"Show All Bookmarks" is almost invisible

Expected Results:
High contrast themes should be applied to "Show All Bookmarks"
Summary: "Show All Bookmarks" item of Bookmarks widget dropdown in → "Show All Bookmarks" item of Bookmarks widget dropdown is almost invisible in High contrast themes
[Tracking Requested - why for this release]:
See Also: → 1042263
Is this a regression from Australis?
Flags: needinfo?(alice0775)
Regression window(fx)
Good:
https://hg.mozilla.org/integration/fx-team/rev/ab6e51e115bc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140312153345
Bad:
https://hg.mozilla.org/integration/fx-team/rev/c8a1458bfe7d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140312153843
Pushlog:
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=ab6e51e115bc&tochange=c8a1458bfe7d

Triggered by:
1a9fa8fde7e0	Darrin Henein — Bug 969592 - Restyle Australis' Bookmarks Menu Popups, r=mak
Blocks: 969592
Flags: needinfo?(alice0775)
Keywords: regression
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
Points: --- → 3
QA Whiteboard: [qa+]
With no progress in the past few weeks, I'm marking 32 as won't fix.

Gavin - Do you think this issue is important enough to track? If so, can you please help get this prioritized into an upcoming iteration in order to address the issue in Firefox 33?
Flags: needinfo?(gavin.sharp)
I don't think we need to track it, but we should fix it soon.

Jared, can you mentor this or suggest someone who can?
Flags: needinfo?(gavin.sharp) → needinfo?(jaws)
We should be able to change the color value from #000 to MenuText at http://hg.mozilla.org/mozilla-central/annotate/4d94eeca89f3/browser/themes/windows/customizableui/panelUIOverlay.css#l45 to fix this bug. Will need some manual testing to verify though.
Mentor: jaws
Flags: needinfo?(jaws)
Whiteboard: [lang=css][good first bug]
Assignee: nobody → abardas
Status: NEW → ASSIGNED
Attached image show-bookmarks-menu.png (obsolete) —
I've also attached a screenshot with the patch applied.
Comment on attachment 8475309 [details] [diff] [review]
bug1042268_show_all_bookmarks_in_bookmarks_widget_dropdown_is_invisible_in_high_contrast_themes.diff

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

Dao, do you see any issues with making this change?
Attachment #8475309 - Flags: review?(jaws) → review?(dao)
Iteration: --- → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
QA Contact: andrei.vaida
Comment on attachment 8475309 [details] [diff] [review]
bug1042268_show_all_bookmarks_in_bookmarks_widget_dropdown_is_invisible_in_high_contrast_themes.diff

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

::: browser/themes/windows/customizableui/panelUIOverlay.css
@@ +41,5 @@
>    padding-top: 4px;
>  }
>  
>  #BMB_bookmarksPopup .menu-text {
> +  color: menutext;

Actually, it looks like this rule can just be deleted. With it deleted, the text will work like the rest of the menuitems. Sorry for the runaround.
Attachment #8475309 - Flags: review?(dao) → review-
That rule is not even applied for osx / linux themes and does the trick, so it's ok to be removed.
Attachment #8475309 - Attachment is obsolete: true
Attachment #8476844 - Flags: review?(dao)
Attachment #8476844 - Flags: review?(dao) → review+
Comment on attachment 8476844 [details] [diff] [review]
Remove css rule which makes 'Show all bookmarks' button hardly visible in High Contrast themes

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

What's the point of the hardcoded color for the disabled state? Shouldn't that be removed just like for the enabled state?
How can I get to a state where "Show all bookmarks" is disabled?
Flags: needinfo?(dao)
I think "subscribe to page" is the only subviewbutton in that menu that gets disabled.
Attached image bookmark-submenu.png
Attachment #8475313 - Attachment is obsolete: true
On windows, there are multiple problems with the bookmarks dropdown menu and all other submenus (see bookmark-submenu.png). I'd like to also fix those submenus in the next patch.

The problem lies here: http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/customizableui/panelUIOverlay.css#26
Flags: needinfo?(dao)
There is another bug which has an attached patch addressing the submenus (bug 1008603), so that part is outside this bug's scope.
Attachment #8476844 - Attachment is obsolete: true
Attachment #8477584 - Flags: review?(dao)
Attachment #8477584 - Flags: review?(dao) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/fcafd91816f7
Keywords: checkin-needed
Whiteboard: [lang=css][good first bug] → [lang=css][good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/fcafd91816f7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=css][good first bug][fixed-in-fx-team] → [lang=css][good first bug]
Target Milestone: --- → Firefox 34
Verified fixed on Nightly 34.0a1 (2014-08-27) using Windows 7 64-bit along with all the available high-contrast themes. 

The sub-menu issue depicted in attachment 8477536 [details] is still visible though, but I suppose that's being taken care of in Bug 1042263.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.