Bug 189687 - Web Inspector: preserve DOM.NodeId if a node is removed and re-added
Summary: Web Inspector: preserve DOM.NodeId if a node is removed and re-added
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: 226624
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-17 20:39 PDT by Devin Rousso
Modified: 2021-06-04 22:11 PDT (History)
14 users (show)

See Also:


Attachments
Patch (21.39 KB, patch)
2018-09-17 22:43 PDT, Devin Rousso
drousso: review-
drousso: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (2.76 MB, application/zip)
2018-09-17 23:33 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (3.33 MB, application/zip)
2018-09-18 00:28 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2018-09-17 20:39:39 PDT
If the same DOM node is removed and then re-added, it is assigned a new nodeId, which breaks the highlighting of any previous "Log Element" or `WI.linkifyNode` values.

* STEPS TO REPRODUCE:
1. Open WebInspector to any page with children of <body>
2. Log any child element of <body> using the context menu "Log Element" (check that it shows `$1` next to the logged value in the console)
3. Remove the logged child element from <body> (e.g. press delete when it's selected)
4. Re-add the removed element by running `document.body.appendChild($1)` in the console
5. Highlight over the previously logged element (the entry with $1)

* EXPECTED:
The re-added node should be highlighted on the page

* ACTUAL:
Nothing is highlighted and an error "Could not find node with given id" is shown in inspector2.
Comment 1 Devin Rousso 2018-09-17 22:43:42 PDT
Created attachment 350002 [details]
Patch

Checking for test failures on EWS while I write tests specific for this patch
Comment 2 EWS Watchlist 2018-09-17 22:45:30 PDT
Attachment 350002 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mark Lam 2018-09-17 22:56:55 PDT
Comment on attachment 350002 [details]
Patch

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

> Source/WebCore/inspector/agents/InspectorCSSAgent.h:148
> -    void didRemoveDOMNode(Node&, int nodeId) override;
> +    void domNodeDestroyed(Node&, int nodeId) override;

nit: is there a reason to not stick with the existing convention and name this "didDestroyDOMNode"?  Also see didModifyDOMAttr below?
Comment 4 Daniel Bates 2018-09-17 23:02:03 PDT
(In reply to Build Bot from comment #2)
> Attachment 350002 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and
> either add and list tests, or explain why no new tests were possible. 
> [changelog/nonewtests] [5]
> Total errors found: 1 in 19 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

This is not a false positive
Comment 5 EWS Watchlist 2018-09-17 23:33:27 PDT
Comment on attachment 350002 [details]
Patch

Attachment 350002 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9252827

New failing tests:
inspector/dom/shadow-and-non-shadow-children.html
inspector/canvas/create-context-bitmaprenderer.html
inspector/console/webcore-logging.html
inspector/canvas/create-context-webgl.html
Comment 6 EWS Watchlist 2018-09-17 23:33:29 PDT
Created attachment 350005 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-09-18 00:28:51 PDT
Comment on attachment 350002 [details]
Patch

Attachment 350002 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9253346

New failing tests:
inspector/canvas/create-context-bitmaprenderer.html
inspector/console/webcore-logging.html
inspector/canvas/create-context-webgl.html
Comment 8 EWS Watchlist 2018-09-18 00:28:53 PDT
Created attachment 350008 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 9 Radar WebKit Bug Importer 2018-09-19 21:08:34 PDT
<rdar://problem/44628244>
Comment 10 Devin Rousso 2018-10-01 11:29:14 PDT
Comment on attachment 350002 [details]
Patch

r- this is not in a state ready for review.