Closed Bug 1456779 Opened 2 years ago Closed 6 months ago

"Computed" view value does not get updated when 3-pane-inspector is on

Categories

(DevTools :: Inspector, defect, P3)

61 Branch
defect

Tracking

(firefox61 wontfix, firefox71 fixed)

RESOLVED FIXED
Firefox 71
Tracking Status
firefox61 --- wontfix
firefox71 --- fixed

People

(Reporter: nachtigall, Assigned: daisuke)

References

(Blocks 1 open bug)

Details

(Whiteboard: [dt-q])

Attachments

(3 files, 1 obsolete file)

The Computed view does not get updated any more, when fiddling with values in the Rules view when the 3-Pane inspector is on. When the 3-Pan inspector is toggled off, then it works (probably because a click on the "Computed" view just updates which does not happen when it's already visible)

STR:

1. Go to https://codepen.io/nachtigall/pen/RyRovV with Nightly and 3-pane inspector on. So have "Rules" and "Computed" visible.
2. Select `.floating` in the Rules view
3. See `float` being `right` in the Computed rule 
4. In the Rules view change `.floating` to `left`

AR:
Computed still shows being the old value `right`

ER:
Computed should update to the new value `left`

Interestingly, the updating works for the `Layout` view.
Assignee: nobody → gl
Blocks: 1433716
Status: NEW → ASSIGNED
Priority: -- → P3
Product: Firefox → DevTools
Comment on attachment 8985947 [details]
Bug 1456779 - Computed view should refresh on reflows.

https://reviewboard.mozilla.org/r/251426/#review257842

::: devtools/client/inspector/computed/computed.js:1481
(Diff revision 1)
>      }
>  
>      const isInactive = !this.isSidebarActive() &&
>                       this.inspector.selection.nodeFront;
>      if (isInactive) {
> +      this.inspector.reflowTracker.untrackReflows(this, this.refresh);

You should also do this in the `destroy` function.
Attachment #8985947 - Flags: review?(pbrosset) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e2eaa601a6d
Computed view should refresh on reflows. r=pbro
Backed out changeset 5e2eaa601a6d (bug 1456779) for devtools failures at devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/6d6d308fb49d157ffde75c8ed7523470075bb6f0

Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5e2eaa601a6d98b5c6d892dc1a6f409145edef7e

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=183853540&repo=mozilla-inbound&lineNumber=1996

20:21:51     INFO -  273 INFO TEST-PASS | devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js | undefined assertion name -
20:21:51     INFO -  274 INFO Testing a matched rule selector (parentmatch)
20:21:51     INFO -  Buffered messages finished
20:21:51    ERROR -  275 INFO TEST-UNEXPECTED-FAIL | devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js:140 - TypeError: nodeInfo is null
20:21:51     INFO -  Stack trace:
20:21:51     INFO -  assertNodeInfo@chrome://mochitests/content/browser/devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js:140:7
20:21:51     INFO -  @chrome://mochitests/content/browser/devtools/client/inspector/computed/test/browser_computed_getNodeInfo.js:176:5
20:21:51     INFO -  Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1098:34
20:21:51     INFO -  async*Tester_execTest@chrome://mochikit/content/browser-test.js:1089:16
20:21:51     INFO -  nextTest/<@chrome://mochikit/content/browser-test.js:991:9
20:21:51     INFO -  SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
20:21:51     INFO -  276 INFO Leaving test bound
20:21:52     INFO -  277 INFO Removing tab.
20:21:52     INFO -  278 INFO Waiting for event: 'TabClose' on [object XULElement].
20:21:52     INFO -  279 INFO Got event: 'TabClose' on [object XULElement].
20:21:52     INFO -  280 INFO Tab removed and finished closing
Flags: needinfo?(gl)
Flags: needinfo?(gl)
Whiteboard: [dt-q]
Since this already has a reviewed+ patch and just a test failure I wonder if this has just been forgotten... Work on this bug looks like 80% done and would be sad if it bitrottens. Gabriel, any chance you could see why the test fails?
Flags: needinfo?(gl)

This bug has not been updated in the last 3 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Assignee: gl → nobody
Status: ASSIGNED → NEW

Haven't had a chance to work on this. The work is mostly done except for figuring out the test failure.

Flags: needinfo?(gl)
Duplicate of this bug: 1497946
Duplicate of this bug: 1579155

:jdescottes analyzed the problem and provided a useful summary in bug 1579155 comment 1.

My analysis is that the update of the computed view here should be triggered from a stylesheet-updated event.
However this event is not emitted by the pageStyle actor when the update comes from the RuleView, because it
triggers an update of type UPDATE_PRESERVING_RULES. And only style updates with type UPDATE_GENERAL are
turned into stylesheet-updated events [...]

Please see the entire comment in the other bug for more info.

Worth noting that the STR in bug 1579155 mentions changing a color. Doing so most probably won't cause a reflow. So, while the approach in the original patch here will work for properties that do cause reflows, it won't work for colors, etc.

Assignee: nobody → daisuke
Status: NEW → ASSIGNED
Attachment #9093213 - Attachment is obsolete: true
Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb1556ff54d6
Update the computed view when the 3 pane rule view is updated. r=daisuke
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3918541135bd
Add test for the computed view updating. r=gl
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
QA Whiteboard: [qa-71b-p2]
You need to log in before you can comment on or make changes to this bug.