Bug 144231 – Web Inspector: Restoring the last selected DOM node fails on reload (DOMAgent: No node with given path found)

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Timothy Hatcher]

DOMTreeManager.js:109:30: CONSOLE ERROR Error during DOMAgent operation: No node with given path found

[Radar WebKit Bug Importer]

<rdar://problem/20702160>

[Matt Baker]

Regressed in http://trac.webkit.org/changeset/183816.

Restoring the last selected DOM node works after backing out the DOMTreeContentView  changes, but the DOMTreeManager console error still occurs.

[Matt Baker]

Created attachment 253856 [details]
[Patch] WIP

[Timothy Hatcher]

Comment on attachment 253856 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=253856&action=review

> Source/WebInspectorUI/ChangeLog:9
> +        Removed _restoreSelectedNodeIsAllowed checks from DOMTreeContentView. The DOMTree is invalidated twice on page
> +        reload, and the content view was preventing the last selected node from being reselected the second time.

I am trying to remember what the bug I was fixing was when I added this. I think I was trying to prevent a refuter restore from clobbering the Inspect Element element if you inspect from the context menu to open the Inspector. Does that work? Or does it jump from the inspected element to the pervasively selected (restored) element?

> Source/WebInspectorUI/ChangeLog:15
> +        (WebInspector.DOMTreeContentView): Deleted.
> +        (WebInspector.DOMTreeContentView.prototype.selectAndRevealDOMNode): Deleted.

These were not deleted.

[Matt Baker]

Comment on attachment 253856 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=253856&action=review

>> Source/WebInspectorUI/ChangeLog:9
>> +        reload, and the content view was preventing the last selected node from being reselected the second time.
> 
> I am trying to remember what the bug I was fixing was when I added this. I think I was trying to prevent a refuter restore from clobbering the Inspect Element element if you inspect from the context menu to open the Inspector. Does that work? Or does it jump from the inspected element to the pervasively selected (restored) element?

I'll look into this.

>> Source/WebInspectorUI/ChangeLog:15
>> +        (WebInspector.DOMTreeContentView.prototype.selectAndRevealDOMNode): Deleted.
> 
> These were not deleted.

Not sure why the change log was generated that way, I'll clean it up by hand.

[Matt Baker]

Comment on attachment 253856 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=253856&action=review

>> Source/WebInspectorUI/ChangeLog:9
>> +        reload, and the content view was preventing the last selected node from being reselected the second time.
> 
> I am trying to remember what the bug I was fixing was when I added this. I think I was trying to prevent a refuter restore from clobbering the Inspect Element element if you inspect from the context menu to open the Inspector. Does that work? Or does it jump from the inspected element to the pervasively selected (restored) element?

I'll look into this.

>> Source/WebInspectorUI/ChangeLog:15
>> +        (WebInspector.DOMTreeContentView.prototype.selectAndRevealDOMNode): Deleted.
> 
> These were not deleted.

Not sure why the change log was generated that way, I'll clean it up by hand.

[Matt Baker]

Comment on attachment 253856 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=253856&action=review

>>>> Source/WebInspectorUI/ChangeLog:9
>>>> +        reload, and the content view was preventing the last selected node from being reselected the second time.
>>> 
>>> I am trying to remember what the bug I was fixing was when I added this. I think I was trying to prevent a refuter restore from clobbering the Inspect Element element if you inspect from the context menu to open the Inspector. Does that work? Or does it jump from the inspected element to the pervasively selected (restored) element?
>> 
>> I'll look into this.
> 
> I'll look into this.

I tried the following and wasn't able to reproduce an issue:

1. Inspect Element via content menu
2. Close Inspector
3. Inspect a new element via context menu
  => Inspector opens, correct node is selected.

[Matt Baker]

The Inspector receives two documentUpdated events from the DOM Agent, and attempts to push the last selected node to the frontend in response to both events. The backend sends the first documentUpdated as soon as the frame's document is set. The DOMTreeManager console error occurs while handling this first event.

The second documentUpdated is sent once the document has finished parsing and DOM content has loaded, at which time the node exists and can be pushed to the frontend.

[Timothy Hatcher]

So it sounds like the error is expected in the first update when most of the DOM isn't populated yet. I wonder if we can suppress that error in that case?

[Matt Baker]

Created attachment 253899 [details]
[Patch] Proposed Fix

[WebKit Commit Bot]

Comment on attachment 253899 [details]
[Patch] Proposed Fix

Rejecting attachment 253899 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 253899, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ubmit
    return self.open(self.click(*args, **kwds))
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 203, in open
    return self._mech_open(url, data, timeout=timeout)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_mechanize.py", line 255, in _mech_open
    raise response
webkitpy.thirdparty.autoinstalled.mechanize._response.httperror_seek_wrapper: HTTP Error 500: Internal Server Error

Full output: http://webkit-queues.appspot.com/results/5663146568056832

[WebKit Commit Bot]

Comment on attachment 253899 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 253899

Committed r184993: <http://trac.webkit.org/changeset/184993>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.