Closed Bug 1440877 Opened 7 years ago Closed 7 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
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).
Status: ASSIGNED → RESOLVED
Closed: 7 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.

Attachment

General

Created:
Updated:
Size: