Closed Bug 1266733 Opened 8 years ago Closed 8 years ago

Attributes of selected element are barely readable in Firebug theme

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(firefox48 fixed, firefox49 fixed)

VERIFIED FIXED
Firefox 49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: sebo, Assigned: Honza)

References

Details

Attachments

(2 files, 1 obsolete file)

When a node is selected within the Inspector panel while using the Firebug theme, the attributes are not readable.

All text within the selected element should be shown in white letters instead (including the arrow brackets of the tags).

Sebastian
Attached patch bug1266733.patch (obsolete) — Splinter Review
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Attachment #8744914 - Flags: review?(bgrinstead)
Comment on attachment 8744914 [details] [diff] [review]
bug1266733.patch

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

::: devtools/client/themes/markup.css
@@ +292,5 @@
>  .theme-selected ~ .editor .theme-fg-color7 {
>    color: var(--theme-selection-color);
>  }
>  
> +/* Make sure even text nodes are white when selected in the Inspector panel. */

Not a big fan of these overrides in general and I'd rather see :not(.theme-selected) added into the selectors above, but that looks complicated due to the sibling selector so I guess this works.

I don't think .editable is really needed in this list.  That element should nested under .open so it should inherit the right color.  Unless if there's a reason for it to be there, please remove it.
Attachment #8744914 - Flags: review?(bgrinstead) → review+
Attached patch bug1266733.patchSplinter Review
(In reply to (Unavailable until April 25) Brian Grinstead [:bgrins] from comment #2)
> I don't think .editable is really needed in this list.  That element should
> nested under .open so it should inherit the right color.
True, removed.

Honza
Attachment #8744914 - Attachment is obsolete: true
Attachment #8744933 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/62646acf6872
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Looks good to me. Thanks for the quick fix!

Sebastian
Status: RESOLVED → VERIFIED
Comment on attachment 8744933 [details] [diff] [review]
bug1266733.patch

Approval Request Comment
[Feature/regressing bug #]: Firebug theme
[User impact if declined]: Unreadable text on selected inspector item
[Describe test coverage new/current, TreeHerder]: Verified in Nightly
[Risks and why]: Low, CSS only
[String/UUID change made/needed]: none
Attachment #8744933 - Flags: approval-mozilla-aurora?
Comment on attachment 8744933 [details] [diff] [review]
bug1266733.patch

Firebug theme related and the fix was verified on Nightly, Aurora48+
Attachment #8744933 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.