Bug 123516 - Web Inspector: Moving an element while in the DOMNodeRemoved handler will hide it in the inspector
Summary: Web Inspector: Moving an element while in the DOMNodeRemoved handler will hid...
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: Alexandru Chiculita
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-10-30 10:57 PDT by Alexandru Chiculita
Modified: 2013-11-05 18:13 PST (History)
7 users (show)

See Also:


Attachments
Test case (651 bytes, text/html)
2013-10-30 10:59 PDT, Alexandru Chiculita
no flags Details
Patch V1 (20.58 KB, patch)
2013-11-05 13:49 PST, Alexandru Chiculita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Chiculita 2013-10-30 10:57:27 PDT
1. Open the attached example.
2. Open the inspector.
3. Make sure the #final_container element is visible in the DOM tree.
4. Click the "Test" button.

Expected Result:
#element should move under the #final_container.

Actual Result:
#element disappears from the DOM tree.

The test attaches to #element's DOMNodeRemoved event. When the handler is triggered it will move the element inside #final_container.
Comment 1 Radar WebKit Bug Importer 2013-10-30 10:57:40 PDT
<rdar://problem/15352934>
Comment 2 Alexandru Chiculita 2013-10-30 10:59:08 PDT
Created attachment 215526 [details]
Test case
Comment 3 Alexandru Chiculita 2013-10-30 10:59:59 PDT
This is the error I see in the console.

Release/WebInspectorUI.framework/Resources/DOMNode.js:553: JS ERROR: TypeError: undefined is not an object (evaluating 'node.parentNode = null')
Comment 4 Timothy Hatcher 2013-10-30 13:23:45 PDT
Nice catch!
Comment 5 Alexandru Chiculita 2013-10-30 14:00:21 PDT
(In reply to comment #4)
> Nice catch!

The problem is that the API will send two removal notifications for the same node. The first one will "unbind" it, so the second one will just have a nodeId === 0.

I guess we need to fix both the API and the JS side as the inspector should be backwards compatible.

Blink seems to have the same issue, just that their JS will recover from the error. The next message will just reinsert #final_container and all its children, so the #element gets fixed.

The content flow API I've just added has the same issue. It sends a nodeId === 0 when the node is removed from the DOM. That's because the node is unbound first and then the new API will just fallback to nodeId === 0.

I've just filed another one related to iframes. Nothing to do with the inspector. https://bugs.webkit.org/show_bug.cgi?id=123529

It all seems to be related to the fact that the node removal notification comes before related JS events execute. The C++ after it will abort the removal if the node has changed the parent, but, there are a couple of things that still happen anyway. For example frames are disconnected even though the element was just moved somewhere else.
Comment 6 Alexandru Chiculita 2013-11-05 13:49:05 PST
Created attachment 216073 [details]
Patch V1

Adding fix for the protocol issue.
Comment 7 WebKit Commit Bot 2013-11-05 18:13:53 PST
Comment on attachment 216073 [details]
Patch V1

Clearing flags on attachment: 216073

Committed r158708: <http://trac.webkit.org/changeset/158708>
Comment 8 WebKit Commit Bot 2013-11-05 18:13:55 PST
All reviewed patches have been landed.  Closing bug.