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)
59 bytes, text/x-review-board-request
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
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
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 email@example.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).
(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.