Firebug theme: Improve storage inspector

RESOLVED FIXED in Firefox 48

Status

enhancement
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: Honza, Assigned: Honza)

Tracking

unspecified
Firefox 49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed, firefox49 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

There are some inconsistencies in the Storage inspector panel when Firebug theme is active. E.g. the selected item in the Storage inspector has white text on white background.

Honza
Assignee: nobody → odvarko
Posted patch bug1266716.patch (obsolete) — Splinter Review
- Fixed vertical centering of hader labels
- Fixed color and bk-color of selected row

Honza
Attachment #8744322 - Flags: review?(ntim.bugs)
Comment on attachment 8744322 [details] [diff] [review]
bug1266716.patch

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

::: devtools/client/themes/widgets.css
@@ +1377,2 @@
>    background: #FFFFFF;
>  }

Instead of these 2 rules, you can simply override --table-zebra-background at the top of this file to #efefef, since the background is already #fff by default.
Attachment #8744322 - Flags: review?(ntim.bugs)
(In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #2)
> Comment on attachment 8744322 [details] [diff] [review]
> bug1266716.patch
> 
> Review of attachment 8744322 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/themes/widgets.css
> @@ +1377,2 @@
> >    background: #FFFFFF;
> >  }
> 
> Instead of these 2 rules, you can simply override --table-zebra-background
> at the top of this file to #efefef, since the background is already #fff by
> default.
I don't understand what you mean. The background should
*not* be #fff if the row is selected.

Honza
Flags: needinfo?(ntim.bugs)
(In reply to Jan Honza Odvarko [:Honza] from comment #3)
> (In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #2)
> > Comment on attachment 8744322 [details] [diff] [review]
> > bug1266716.patch
> > 
> > Review of attachment 8744322 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: devtools/client/themes/widgets.css
> > @@ +1377,2 @@
> > >    background: #FFFFFF;
> > >  }
> > 
> > Instead of these 2 rules, you can simply override --table-zebra-background
> > at the top of this file to #efefef, since the background is already #fff by
> > default.
> I don't understand what you mean. The background should
> *not* be #fff if the row is selected.
> 
> Honza

The background is already #fff for unstriped rows.
Flags: needinfo?(ntim.bugs)
Posted patch bug1266716.patch (obsolete) — Splinter Review
Hm..., I am not fully sure what changed (my feeling is that it could be inline-editing for the Storage panel), but it looks like these two rows aren't necessary anymore. One change to the splitter though.

Honza
Attachment #8744322 - Attachment is obsolete: true
Attachment #8745323 - Flags: review?(ntim.bugs)
Severity: normal → enhancement
Comment on attachment 8745323 [details] [diff] [review]
bug1266716.patch

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

r=me with this comment addressed.

::: devtools/client/themes/widgets.css
@@ +1368,2 @@
>    position: absolute;
>  }

You might want to remove this whole block (.theme-firebug .table-widget-body .devtools-side-splitter) alltogether. It seems to be an improvement for the firebug theme (since it makes it look more consistent with the netmonitor by adding separators to the header).
Attachment #8745323 - Flags: review?(ntim.bugs) → review+
(In reply to Tim Nguyen :ntim from comment #6)
> Comment on attachment 8745323 [details] [diff] [review]
> bug1266716.patch
> 
> Review of attachment 8745323 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with this comment addressed.
Done, it looks great now!

Honza
Attachment #8745323 - Attachment is obsolete: true
Attachment #8745980 - Flags: review+
Comment on attachment 8745980 [details] [diff] [review]
bug1266716.patch

Approval Request Comment
[Feature/regressing bug #]: New Firebug theme
[User impact if declined]: Unreadable text for selected rows in Storage inspector
[Describe test coverage new/current, TreeHerder]: soon in Nightly, locally tested
[Risks and why]: Low, CSS code removal
[String/UUID change made/needed]: none
Attachment #8745980 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1267796
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/7b4589f13fca
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Comment on attachment 8745980 [details] [diff] [review]
bug1266716.patch

Firebug theme related, Aurora48+
Attachment #8745980 - 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.