Closed
Bug 1517187
Opened 6 years ago
Closed 6 years ago
Rulers and Measure buttons should have a visible active state
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(firefox-esr60 unaffected, firefox64 unaffected, firefox65 unaffected, firefox66 fixed)
RESOLVED
FIXED
Firefox 66
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox64 | --- | unaffected |
firefox65 | --- | unaffected |
firefox66 | --- | fixed |
People
(Reporter: fvsch, Assigned: yulia)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
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.
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Weird, I'm pretty sure they worked at some point. Can you try finding a regression range using https://mozilla.github.io/mozregression/ ?
EDIT: done in comment 3
status-firefox64:
--- → ?
status-firefox65:
--- → ?
status-firefox66:
--- → affected
status-firefox-esr60:
--- → ?
Keywords: regression,
regressionwindow-wanted
Comment 2•6 years ago
|
||
Seems to work on beta, so it must be a recent regression.
Comment 3•6 years ago
|
||
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:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=69649540a938d1f3d9f145f34d074932a3feff4e&tochange=7bbdead1fa31f67c086340991707b62160b7c66a
Hi Yulia, your patch (bug 1508660) seems to cause this regression. Could you please look into this ?
Blocks: 1508660
Flags: needinfo?(ystartsev)
Updated•6 years ago
|
Keywords: regressionwindow-wanted
Comment 4•6 years ago
|
||
Not sure if that helps, but this is the code that sets the checked state for the rulers/measure command buttons: https://searchfox.org/mozilla-central/rev/ecf61f8f3904549f5d65a8a511dbd7ea4fd1a51d/devtools/client/definitions.js#596-609
Assignee | ||
Comment 5•6 years ago
|
||
This fixes an issue with the buttons and also adds a test
Assignee | ||
Comment 6•6 years ago
|
||
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 ystartsev@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b70afc7d2c99
fix rulers and measure buttons to display state; r=jdescottes
Comment 8•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Updated•6 years ago
|
Assignee: nobody → ystartsev
You need to log in
before you can comment on or make changes to this bug.
Description
•