Closed Bug 1306139 Opened 8 years ago Closed 8 years ago

Firebug theme - Body color needs to be adjusted

Categories

(DevTools :: General, defect, P3)

52 Branch
defect

Tracking

(firefox52 verified)

VERIFIED FIXED
Firefox 52
Tracking Status
firefox52 --- verified

People

(Reporter: sebo, Assigned: xfq)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=css])

Attachments

(6 files)

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
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)?
Attached image 18191a.png
Attached image 000000.png
Priority: -- → P3
(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.
(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 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+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/328c7a2f6040
Fix the body color of the Firebug theme r=ntim
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)
Here's a screenshot of the tooltip.

Sebastian
(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)
(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
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
Attached image after-changing-js.png
I attached two screenshots of the font tooltip (the name of the images should be self-explanatory).
https://hg.mozilla.org/mozilla-central/rev/328c7a2f6040
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
(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: → 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
Status: RESOLVED → VERIFIED
Thank you for the verification, :NaSb!
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: