Closed Bug 1517187 Opened 2 years ago Closed 2 years ago

Rulers and Measure buttons should have a visible active state


(DevTools :: General, defect)

Not set


(firefox-esr60 unaffected, firefox64 unaffected, firefox65 unaffected, firefox66 fixed)

Firefox 66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed


(Reporter: fvsch, Assigned: yulia)


(Blocks 1 open bug)


(Keywords: regression)


(1 file)

The Rulers and Measure features have on/off states, but their buttons do not show an "active" state (with a blue icon), unlike other buttons: Paint Flashing, RDM, Frames, Pick an Element.

We should use the same "active" style to provide users with visual feedback that the feature is enabled.

Somewhat related: bug 1260225 "Move inspector-related command buttons (rulers, measurement) to the Inspector panel and enable them by default"
If we land bug 1260225 first, then we should make sure we use active states there.
See Also: → 1260225
Blocks: 1444299
Weird, I'm pretty sure they worked at some point. Can you try finding a regression range using ?

EDIT: done in comment 3
Seems to work on beta, so it must be a recent regression.
12:02.30 INFO: Narrowed inbound regression window from [69649540, ae0f6d53] (3 builds) to [69649540, 7bbdead1] (2 builds) (~1 steps left)
12:02.30 INFO: No more inbound revisions, bisection finished.
12:02.30 INFO: Last good revision: 69649540a938d1f3d9f145f34d074932a3feff4e
12:02.30 INFO: First bad revision: 7bbdead1fa31f67c086340991707b62160b7c66a
12:02.30 INFO: Pushlog:

Hi Yulia, your patch (bug 1508660) seems to cause this regression. Could you please look into this ?
Blocks: 1508660
Flags: needinfo?(ystartsev)
Not sure if that helps, but this is the code that sets the checked state for the rulers/measure command buttons:
This fixes an issue with the buttons and also adds a test
The issue is related to some of the fission work that is going on. I forgot that inspector is still a special case when updating this code. The fix was trivial, but there will be follow up work once we fix the inspector destruction issue (mentioned in the todo of the patch).

One thing I am not too sure about is how I handled the tests. Since we are not using redux in this part of the code, it makes it difficult to detect changes to the UI. Clicking the button doesn't result in a reflow right away. I used a mutation observer until we migrate this bit of the code. We might have a better pattern -- if anyone here has an idea. I will also ask around.
Flags: needinfo?(ystartsev)
Pushed by
fix rulers and measure buttons to display state; r=jdescottes
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Assignee: nobody → ystartsev
You need to log in before you can comment on or make changes to this bug.