Bug 110424

Summary:

Web Inspector: Cannot deep expand an element that has previously been partially expanded

Product:

WebKit

Reporter:

Antoine Quint <graouts>

Component:

Web Inspector (Deprecated)

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, joepeck, keishi, loislo, pfeldman, pmuellr, timothy, vsevik, web-inspector-bugs, webkit-bug-importer, yurys

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Work-in-progress patch, no tests yet.	none
Patch	pfeldman: review+

Antoine Quint

Reported 2013-02-20 18:37:47 PST

While we added support for true deep-expansion with https://bugs.webkit.org/show_bug.cgi?id=66868, there is (at least) one case where this fails. In InspectorDOMAgent::pushChildNodesToFrontend(), if we find that we'd already requested children for the given node, we return early: if (m_childrenRequested.contains(nodeId)) return; This could easily happen for the <body> or <html> elements, or basically any element that you inspect directly to open the inspector.

Attachments
Work-in-progress patch, no tests yet. (1.14 KB, patch) 2013-02-20 18:56 PST, Antoine Quint	no flags	Details Formatted Diff Diff
Patch (7.33 KB, patch) 2013-02-25 23:14 PST, Antoine Quint	pfeldman: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2013-02-20 18:38:26 PST

<rdar://problem/13260224>

Antoine Quint

Comment 2 2013-02-20 18:56:44 PST

Created attachment 189445 [details] Work-in-progress patch, no tests yet.

Pavel Feldman

Comment 3 2013-02-20 22:44:29 PST

Comment on attachment 189445 [details] Work-in-progress patch, no tests yet. View in context: https://bugs.webkit.org/attachment.cgi?id=189445&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:453 > + node = node->firstChild(); for (node = innerFirstChild(node); node; node = innerNextSibling(node)) { ... > Source/WebCore/inspector/InspectorDOMAgent.cpp:455 > + nodeId = m_documentNodeToIdMap.get(node); Dangling nodes (the ones printed with console.log(document)) will end up with nodes being a part of another NodeToIdMap instance. m_idToNodesMap knows which map to use. Please do not reuse nodeId, this one is about child node. childNodeId > Source/WebCore/inspector/InspectorDOMAgent.cpp:456 > + if (nodeId) Should be an assertion. We know that m_childrenRequested.contains(nodeId). Hence we've sent its children.

Antoine Quint

Comment 4 2013-02-25 22:22:31 PST

(In reply to comment #3) > (From update of attachment 189445 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189445&action=review > > > Source/WebCore/inspector/InspectorDOMAgent.cpp:455 > > + nodeId = m_documentNodeToIdMap.get(node); > > Dangling nodes (the ones printed with console.log(document)) will end up with nodes being a part of another NodeToIdMap instance. m_idToNodesMap knows which map to use. Should I simply move the `NodeToIdMap* nodeMap = m_idToNodesMap.get(nodeId);` call to before I enter the `m_childrenRequested.contains(nodeId)` condition and used this nodeMap to get the childNodeId?

Antoine Quint

Comment 5 2013-02-25 23:14:33 PST

Created attachment 190216 [details] Patch

Pavel Feldman

Comment 6 2013-02-26 00:36:14 PST

Comment on attachment 190216 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190216&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:457 > + childNodeId = nodeMap->get(node); you never use childNodeId outside for scope - declare it inline.

Antoine Quint

Comment 7 2013-02-26 08:00:12 PST

(In reply to comment #6) > (From update of attachment 190216 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=190216&action=review > > > Source/WebCore/inspector/InspectorDOMAgent.cpp:457 > > + childNodeId = nodeMap->get(node); > > you never use childNodeId outside for scope - declare it inline. Made the change in commit. Thanks for the review Pavel.

Antoine Quint

Comment 8 2013-02-26 08:00:34 PST

Landed in http://trac.webkit.org/changeset/144057.

Note You need to log in before you can comment on or make changes to this bug.