Closed Bug 1408198 Opened 4 years ago Closed 4 years ago

Inspector breadcrumbs scrollbutton is black in dark theme

Categories

(DevTools :: General, defect, P2)

58 Branch
defect

Tracking

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 unaffected, firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: euthanasia_waltz, Assigned: jdescottes)

References

Details

(Keywords: regression)

Attachments

(3 files)

Thank you for filing this regression. It indeed works in 57, but doesn't in 58. mozregression indicates bug 1399886 as being the culprit.
Blocks: 1399886
Severity: minor → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Priority: -- → P2
Summary: devtools/inspector, scrollbutton is black in dark theme → Inspector breadcrumbs scrollbutton is black in dark theme
The scrollbuttons icons are currently PNG, but we can replace them with the photon ones which are SVG: http://design.firefox.com/icons/viewer/#arrowhead
Do we wanna block 58 on this? Not much time left in the 58 window.
Flags: needinfo?(ntim.bugs)
Taking this one.
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Ended up using "arrowhead left 16" from the Photon icons. "arrowhead left 12" was too thick, and stood out too much IMO. 

Screenshot with both light and dark themes.
Comment on attachment 8926475 [details]
Bug 1408198 - Use SVG icon for breadcrumbs scrollbuttons;

Clearing, needs some more work for firebug theme
Attachment #8926475 - Flags: review?(ntim.bugs)
Comment on attachment 8926475 [details]
Bug 1408198 - Use SVG icon for breadcrumbs scrollbuttons;

https://reviewboard.mozilla.org/r/197714/#review202940

Thanks for working on this! It seems that you forgot to add the SVG in the patch :)
Attachment #8926475 - Flags: review?(ntim.bugs)
Seems like this can be landed in time for 58.
Flags: needinfo?(ntim.bugs)
(In reply to Tim Nguyen :ntim from comment #9)
> Comment on attachment 8926475 [details]
> Bug 1408198 - Use SVG icon for breadcrumbs scrollbuttons;
> 
> https://reviewboard.mozilla.org/r/197714/#review202940
> 
> Thanks for working on this! It seems that you forgot to add the SVG in the
> patch :)

Oops, updated.
Comment on attachment 8926475 [details]
Bug 1408198 - Use SVG icon for breadcrumbs scrollbuttons;

https://reviewboard.mozilla.org/r/197714/#review202952

::: devtools/client/themes/breadcrumbs.css:31
(Diff revision 3)
>  .scrollbutton-down > .toolbarbutton-icon {
>    -moz-appearance: none;
> -  width: 7px;
> +  width: 20px;
>    height: 16px;
> -  background-size: 14px 16px;
> -  background-position: 0 center;
> +  background-size: 16px;
> +  background-position: 2px center;

background-position: center; just works fine, no need for the magic `2px` ;)

::: devtools/client/themes/breadcrumbs.css:236
(Diff revision 3)
>  .theme-firebug .breadcrumbs-widget-container .scrollbutton-up:not([disabled]):active:hover > .toolbarbutton-icon,
>  .theme-firebug .breadcrumbs-widget-container .scrollbutton-down:not([disabled]):active:hover > .toolbarbutton-icon {
> -  background-position: 0 center;
> +  background-position: 2px center;
>  }

This rule doesn't seem to be useful anymore (neither in the old debugger, neither in the inspector). Can you remove it?
Attachment #8926475 - Flags: review?(ntim.bugs) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77d2425c4b40
Use SVG icon for breadcrumbs scrollbuttons;r=ntim
https://hg.mozilla.org/mozilla-central/rev/77d2425c4b40
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.