Firebug theme - Body color needs to be adjusted

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools
P3
minor
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: sebo, Assigned: xfq)

Tracking

(Blocks: 1 bug, {good-first-bug})

52 Branch
Firefox 52
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox52 verified)

Details

(Whiteboard: [lang=css])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments)

(Reporter)

Description

a year ago
The Firebug theme currently uses #18191a[1] as the body color, which is used in different parts of the UI like e.g. the font tooltip.

In Firebug 2.0 this color was pure black, i.e. #000000, so it should be changed accordingly.

Sebastian

[1] https://dxr.mozilla.org/mozilla-central/rev/66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/devtools/client/themes/variables.css#150
(Assignee)

Comment 1

a year ago
Hi,

I do not know DevTools and the Firefox code base very well and this is my first attempt at fixing a DevTools bug (to get a good sense of how DevTools works).

I'm not sure if I've understood this bug correctly. Here are what I just tried:

1. Enable browser chrome debugging toolbox and remote debugging
2. Switch to the Firebug theme
3. Change --theme-body-color to #000000 in the browser toolbox (in the DOM and Style Inspector)

But I failed to find any difference after changing the color. Perhaps because #18191a looks similar to #000000?

I'm not familiar with Mercurial and the fix->review->respond->r+ cycle for Firefox/DevTools.

Can anyone point me in the right direction (for code and/or process)?
(Assignee)

Comment 2

a year ago
Created attachment 8796104 [details]
18191a.png
(Assignee)

Comment 3

a year ago
Created attachment 8796105 [details]
000000.png
Priority: -- → P3

Comment 4

a year ago
(In reply to Xue Fuqiao [:xfq] from comment #1)
> Hi,
> 
> I do not know DevTools and the Firefox code base very well and this is my
> first attempt at fixing a DevTools bug (to get a good sense of how DevTools
> works).
> 
> I'm not sure if I've understood this bug correctly. Here are what I just
> tried:
> 
> 1. Enable browser chrome debugging toolbox and remote debugging
> 2. Switch to the Firebug theme
> 3. Change --theme-body-color to #000000 in the browser toolbox (in the DOM
> and Style Inspector)
> 
> But I failed to find any difference after changing the color. Perhaps
> because #18191a looks similar to #000000?
They are both black, so you likely won't see much difference.

> I'm not familiar with Mercurial and the fix->review->respond->r+ cycle for
> Firefox/DevTools.
> 
> Can anyone point me in the right direction (for code and/or process)?

There's a git based workflow documented here: https://hacks.mozilla.org/2016/02/introducing-devtools-reload/

Once you've done the changes (in devtools/client/themes/variables.css), you can generate a github diff then attach it here.
(Assignee)

Comment 5

a year ago
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #4)
> (In reply to Xue Fuqiao [:xfq] from comment #1)
> > But I failed to find any difference after changing the color. Perhaps
> > because #18191a looks similar to #000000?
> They are both black, so you likely won't see much difference.

I see. Thanks.

> > I'm not familiar with Mercurial and the fix->review->respond->r+ cycle for
> > Firefox/DevTools.
> > 
> > Can anyone point me in the right direction (for code and/or process)?
> 
> There's a git based workflow documented here:
> https://hacks.mozilla.org/2016/02/introducing-devtools-reload/
> 
> Once you've done the changes (in devtools/client/themes/variables.css), you
> can generate a github diff then attach it here.

Thanks for the link.

Since I've already 'hg clone'd mozilla-central and built Firefox, I'll try to learn to use Mercurial.

Will commit the change and 'hg push https://reviewboard-hg.mozilla.org/autoreview' later.
Comment hidden (mozreview-request)

Comment 7

a year ago
mozreview-review
Comment on attachment 8796451 [details]
Bug 1306139 - Fix the body color of the Firebug theme

https://reviewboard.mozilla.org/r/82320/#review80928

Looks good to me, thanks for the patch! I will now trigger autoland, so your patch will be merged.
Attachment #8796451 - Flags: review?(ntim.bugs) → review+

Comment 8

a year ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/328c7a2f6040
Fix the body color of the Firebug theme r=ntim
(Reporter)

Comment 9

a year ago
I didn't have the time to test it myself yet, but Xue, did you check whether it actually changes the font color of the font preview tooltip (within the Rules side panel of the Inspector), which was actually the main reason I filed this issue?

Sebastian
Assignee: nobody → xfq.free
Flags: needinfo?(xfq.free)
(Reporter)

Comment 10

a year ago
Created attachment 8796488 [details]
Font tooltip using wrong color

Here's a screenshot of the tooltip.

Sebastian
(Assignee)

Comment 11

a year ago
(In reply to Sebastian Zartner [:sebo] from comment #9)
> I didn't have the time to test it myself yet, but Xue, did you check whether
> it actually changes the font color of the font preview tooltip (within the
> Rules side panel of the Inspector), which was actually the main reason I
> filed this issue?

Sorry, I didn't check it.

Should I do a './mach build faster', then './mach run' and see the color of the font tooltip?
Flags: needinfo?(xfq.free)
(Reporter)

Comment 12

a year ago
(In reply to Xue Fuqiao [:xfq] from comment #11)
> (In reply to Sebastian Zartner [:sebo] from comment #9)
> > I didn't have the time to test it myself yet, but Xue, did you check whether
> > it actually changes the font color of the font preview tooltip (within the
> > Rules side panel of the Inspector), which was actually the main reason I
> > filed this issue?
> 
> Sorry, I didn't check it.
> 
> Should I do a './mach build faster', then './mach run' and see the color of
> the font tooltip?

You can do it that way or run them as add-on as described here:
https://developer.mozilla.org/en-US/docs/Tools/Contributing/Contribute_on_nightly#Load_Nightly%27s_devtools_from_the_source_code

Sebastian
(Assignee)

Comment 13

a year ago
I just checked it but didn't find anything difference before and after changing '--theme-body-color'.

Then I looked around the code and found the fillStyle[1] for drawing the font preview canvas:

* https://dxr.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/devtools/server/actors/inspector.js#695
* https://dxr.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/devtools/server/actors/styles.js#32

But the color of the font tooltip still didn't change after I changed the fillStyles in these two files and rebuilt Firefox. Am I missing something?

[1] https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/fillStyle
(Assignee)

Comment 14

a year ago
Created attachment 8796572 [details]
after-changing-theme-body-color.png
(Assignee)

Comment 15

a year ago
Created attachment 8796573 [details]
after-changing-js.png
(Assignee)

Comment 16

a year ago
I attached two screenshots of the font tooltip (the name of the images should be self-explanatory).

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/328c7a2f6040
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
(Reporter)

Comment 18

a year ago
(In reply to Xue Fuqiao [:xfq] from comment #13)
> But the color of the font tooltip still didn't change after I changed the
> fillStyles in these two files and rebuilt Firefox. Am I missing something?

I've posted my conclusions in bug 1088305 comment 4.

Sebastian
No longer depends on: 1088305
See Also: → bug 1088305
I've managed this issue on this bug in Nightly 52.0a1 (2016-09-28) (64-bit) from Ubuntu 14.04.5 

This Bug is now verified as fixed on Latest Firefox Nightly 52.0a1 (2016-11-11) (64-bit) 

Build ID: 20161111030203
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
OS: Linux 4.4.0-47-generic; Ubuntu 14.04.5

Updated

a year ago
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
(Assignee)

Comment 20

a year ago
Thank you for the verification, :NaSb!
You need to log in before you can comment on or make changes to this bug.