Bug 144231 - Web Inspector: Restoring the last selected DOM node fails on reload (DOMAgent: No node with given path found)
Summary: Web Inspector: Restoring the last selected DOM node fails on reload (DOMAgent...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks: 145446
  Show dependency treegraph
 
Reported: 2015-04-26 11:19 PDT by Timothy Hatcher
Modified: 2015-05-29 11:06 PDT (History)
8 users (show)

See Also:


Attachments
[Patch] WIP (3.00 KB, patch)
2015-05-28 12:36 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (2.88 KB, patch)
2015-05-29 02:21 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2015-04-26 11:19:02 PDT
DOMTreeManager.js:109:30: CONSOLE ERROR Error during DOMAgent operation: No node with given path found
Comment 1 Radar WebKit Bug Importer 2015-04-26 11:19:13 PDT
<rdar://problem/20702160>
Comment 2 Matt Baker 2015-05-28 12:36:02 PDT
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.
Comment 3 Matt Baker 2015-05-28 12:36:48 PDT
Created attachment 253856 [details]
[Patch] WIP
Comment 4 Timothy Hatcher 2015-05-28 14:34:05 PDT
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.
Comment 5 Matt Baker 2015-05-28 14:55:01 PDT
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.
Comment 6 Matt Baker 2015-05-28 14:55:03 PDT
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.
Comment 7 Matt Baker 2015-05-28 16:15:00 PDT
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.
Comment 8 Matt Baker 2015-05-28 16:45:03 PDT
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.
Comment 9 Timothy Hatcher 2015-05-28 18:06:21 PDT
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?
Comment 10 Matt Baker 2015-05-29 02:21:31 PDT
Created attachment 253899 [details]
[Patch] Proposed Fix
Comment 11 WebKit Commit Bot 2015-05-29 08:23:28 PDT
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
Comment 12 WebKit Commit Bot 2015-05-29 11:06:12 PDT
Comment on attachment 253899 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 253899

Committed r184993: <http://trac.webkit.org/changeset/184993>
Comment 13 WebKit Commit Bot 2015-05-29 11:06:19 PDT
All reviewed patches have been landed.  Closing bug.