Closed Bug 1440877 Opened 4 years ago Closed 4 years ago

I found a typo in a CSS variable, "--toolbar-gbimage" (written but never read or mentioned anywhere else)

Categories

(Firefox :: Theme, defect, P5)

58 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: sobeita, Assigned: dao)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180208175058

Steps to reproduce:

Nothing to reproduce, but I found it in the Style Editor and couldn't figure out what it was for.


Actual results:

I searched for all instances of the variable as written, then a shorter substring to see if it could be related to anything other than the typo I suspected, and finally the variable I thought it should be.
https://dxr.mozilla.org/mozilla-central/search?q=toolbar-gbimage --> 2 writes (to 'none')
https://dxr.mozilla.org/mozilla-central/search?q=gbimage -> +58 references to different cases of 'rgbimage'
https://dxr.mozilla.org/mozilla-central/search?q=toolbar-bgimage --> 5 reads, 8 writes

Apparently one of the two instances was introduced in bug 1349555 about 7 months ago.


Expected results:

There should have been at least one use of the variable, most likely "background-image: var(--toolbar-gbimage)", if it were not a typo. In this case it's almost certainly benign, but validation should catch unused variables (and probably one or more undefined references to the correctly-spelled variable since the typo was defined instead.)
Component: Untriaged → Theme
Flags: needinfo?(dao+bmo)
Looks like we never needed to reset the variable in compacttheme.inc.css in the first place -- we already set it to none for all lwthemes.
Assignee: nobody → dao+bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(dao+bmo)
Priority: -- → P5
Comment on attachment 8954705 [details]
Bug 1440877 - Remove mistyped --toolbar-gbimage variable.

https://reviewboard.mozilla.org/r/223826/#review229842
Attachment #8954705 - Flags: review?(jaws) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8da99694cb6
Remove mistyped --toolbar-gbimage variable. r=jaws
Great, thanks. Was that the only instance you found? I saw 2 results, although one may be generated from the other (obj-x86_64-pc-linux-gnu/dist/bin/browser/chrome/browser/skin/classic/browser/compacttheme.css).
https://hg.mozilla.org/mozilla-central/rev/b8da99694cb6
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
(In reply to sobeita from comment #5)
> Great, thanks. Was that the only instance you found? I saw 2 results,
> although one may be generated from the other
> (obj-x86_64-pc-linux-gnu/dist/bin/browser/chrome/browser/skin/classic/
> browser/compacttheme.css).

Yes, exactly.
Thank you for filing this bug! I wanted to let you know that because of your bug I have updated our tooling to try to catch mistakes like this by using an automated test that will run on each check-in. You can see the test changes that I made in bug 1441882.
That's amazing, exactly what I was looking for! Thank you too!
You need to log in before you can comment on or make changes to this bug.