989009 - Australis: Hard to read highlighted label |Open "LiveBookmarks Name"| of LiveBookmarks (Windows7 Classic)

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Alice0775 White]

In Windows7 Classic

Steps To Reproduce:
1. Create a LiveBookmarks
2. Expand the LiveBookmarks from Bookmarks Widget
3. Select |Open "LibeBookmarks Name"| on top of the menupopup

Actual Results:
Hard to read text

Expected Results:
Highlighted text color and background color should be same as other item

Comment 1

[Alice0775 White]

Attachment: screenshot

Comment 2

[:Gijs (he/him)]

This would be P3- but the illegibility makes this worse. :-\

Comment 3

[Alice0775 White]

Attachment: screenshot2

Label "LiveBookmark Loading..." is also affected

Comment 4

[:Gijs (he/him)]

Attachment: should theme live bookmark submenu items correctly,

This is pretty simple. Marco, look OK to you?

Comment 5

[Marco Bonardo [:mak]]

Comment on attachment 8398464 [details] [diff] [review]
should theme live bookmark submenu items correctly,

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

::: browser/components/places/content/browserPlacesViews.js
@@ +394,5 @@
>        // Add "Open (Feed Name)" menuitem.
>        aPopup._siteURIMenuitem = document.createElement("menuitem");
> +      let className = "openlivemarksite-menuitem";
> +      if (typeof this.options.extraClasses.entry == "string") {
> +        className += " " + this.options.extraClasses.entry;

why don't you just use classList.add and leave the current code untouched? should make the patch smaller.

@@ +433,5 @@
>        // Create the status menuitem and cache it in the popup object.
>        statusMenuitem = document.createElement("menuitem");
> +      let className = "livemarkstatus-menuitem";
> +      if (typeof this.options.extraClasses.entry == "string") {
> +        className += " " + this.options.extraClasses.entry;

ditto

Comment 6

[:Gijs (he/him)]

w/ classList.add:

remote:   https://hg.mozilla.org/integration/fx-team/rev/6ed1e0ac4f12

Comment 7

[:Gijs (he/him)]

Comment on attachment 8398464 [details] [diff] [review]
should theme live bookmark submenu items correctly,

(I'm going to be away the coming week, so pre-emptively asking for approval)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis / bookmark menu restyle bugs (bug 969592, bug 969584)
User impact if declined: feed "open feed" and "feed failed to load" items are misstyled everywhere, in some cases they are hard to read (e.g. Windows classic)
Testing completed (on m-c, etc.): local, soon on m-c
Risk to taking this patch (and alternatives if risky): very low, adding a class to these items to bring their styling in line with the other items
String or IDL/UUID changes made by this patch: none

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6ed1e0ac4f12

Comment 9

[Mike de Boer [:mikedeboer]]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/9f3a5a235d09

Comment 10

[Mike de Boer [:mikedeboer]]

Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/59b536a913c7

Comment 11

[Andrei Vaida [:avaida]]

I was able to confirm the fix for this issue on Windows 7 64-bit [1] using:
- the latest Beta (Build ID: 20140331125246),
- the latest Aurora (Build ID: 20140401004001),
- the latest Nightly (Build ID: 20140401030203).

1. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0