- Revision
- 158854
- Author
- [email protected]
- Date
- 2013-11-07 10:05:55 -0800 (Thu, 07 Nov 2013)
Log Message
Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
https://bugs.webkit.org/show_bug.cgi?id=123577
Source/WebCore:
Reviewed by Timothy Hatcher.
Test: inspector-protocol/model/content-flow-content-removal.html
Do not send unregister events for the content nodes of a flow when the element is not part of the DOM
anymore. We already send an unbind event, so the inspector is already notified that the node was removed.
* inspector/InspectorCSSAgent.cpp:
(WebCore::InspectorCSSAgent::didUnregisterNamedFlowContentElement):
Source/WebInspectorUI:
Reviewed by Timothy Hatcher.
Fixed the content node removal from the content flow.
* UserInterface/ContentFlowTreeContentView.js:
* UserInterface/DOMTreeManager.js:
(WebInspector.DOMTreeManager):
(WebInspector.DOMTreeManager.prototype._createContentFlowFromPayload): Registered all the content nodes
in the _contentNodesToFlowsMap.
(WebInspector.DOMTreeManager.prototype._unbind): Added call to _removeContentNodeFromFlowIfNeeded.
(WebInspector.DOMTreeManager.prototype._removeContentNodeFromFlowIfNeeded): Called from _unbind to check
and remove a node from it's parent content flow if needed.
(WebInspector.DOMTreeManager.prototype.unregisteredNamedFlowContentElement):
LayoutTests:
Reviewed by Timothy Hatcher.
Added test to check that the notification that an element was removed from the ContentFlow is handled
correctly in the WebInspector even if the element is not part of the DOM anymore.
* inspector-protocol/model/content-flow-content-removal-expected.txt: Added.
* inspector-protocol/model/content-flow-content-removal.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (158853 => 158854)
--- trunk/LayoutTests/ChangeLog 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/LayoutTests/ChangeLog 2013-11-07 18:05:55 UTC (rev 158854)
@@ -1,3 +1,16 @@
+2013-11-07 Alexandru Chiculita <[email protected]>
+
+ Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+ https://bugs.webkit.org/show_bug.cgi?id=123577
+
+ Reviewed by Timothy Hatcher.
+
+ Added test to check that the notification that an element was removed from the ContentFlow is handled
+ correctly in the WebInspector even if the element is not part of the DOM anymore.
+
+ * inspector-protocol/model/content-flow-content-removal-expected.txt: Added.
+ * inspector-protocol/model/content-flow-content-removal.html: Added.
+
2013-10-24 Jer Noble <[email protected]>
[MSE] Add mock MediaSource classes for testing.
Added: trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal-expected.txt (0 => 158854)
--- trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal-expected.txt 2013-11-07 18:05:55 UTC (rev 158854)
@@ -0,0 +1,9 @@
+Testing that the ContentFlows events are correctly dispatched when content nodes are detached from the DOM.
+
+PASS: ContentFlow was added.
+PASS: ContentFlow.contentNodes has a length of 2.
+PASS: ContentFlow.contentNodes[0].id is "#contentStatic".
+PASS: ContentFlow.contentNodes[1].id is "#contentRemoved".
+PASS: "#contentRemoved" was removed.
+PASS: "#contentRemoved" cannot be found in the contentNodes list.
+
Added: trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal.html (0 => 158854)
--- trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal.html (rev 0)
+++ trunk/LayoutTests/inspector-protocol/model/content-flow-content-removal.html 2013-11-07 18:05:55 UTC (rev 158854)
@@ -0,0 +1,64 @@
+<!doctype html>
+<html>
+<head>
+<style>
+.content
+{
+ -webkit-flow-into: flow;
+}
+</style>
+<script type="text/_javascript_" src=""
+<script>
+function changeFlowContent()
+{
+ document.getElementById("contentRemoved").remove();
+}
+
+function test()
+{
+ InspectorTest.importInspectorScripts();
+
+ var contentFlow;
+
+ WebInspector.frameResourceManager.addEventListener(WebInspector.FrameResourceManager.Event.MainFrameDidChange, function(event) {
+ var domTree = WebInspector.frameResourceManager.mainFrame.domTree;
+ domTree.addEventListener(WebInspector.DOMTree.Event.RootDOMNodeInvalidated, onRootDOMNodeInvalidated, null);
+ domTree.addEventListener(WebInspector.DOMTree.Event.ContentFlowWasAdded, onContentFlowWasAdded, null);
+ domTree.requestContentFlowList();
+ });
+
+ function onRootDOMNodeInvalidated()
+ {
+ WebInspector.frameResourceManager.mainFrame.domTree.requestContentFlowList();
+ }
+
+ function onContentFlowWasAdded(event)
+ {
+ contentFlow = event.data.flow;
+ InspectorTest.assert(contentFlow.name === "flow", "ContentFlow was added.");
+ InspectorTest.assert(contentFlow.contentNodes.length === 2, "ContentFlow.contentNodes has a length of 2.");
+ InspectorTest.assert(contentFlow.contentNodes[0].getAttribute("id") === "contentStatic", "ContentFlow.contentNodes[0].id is \"#contentStatic\".");
+ InspectorTest.assert(contentFlow.contentNodes[1].getAttribute("id") === "contentRemoved", "ContentFlow.contentNodes[1].id is \"#contentRemoved\".");
+
+ contentFlow.addEventListener(WebInspector.ContentFlow.Event.ContentNodeWasRemoved, onContentNodeWasRemoved, null);
+
+ InspectorTest.sendCommand("Runtime.evaluate", {_expression_: "changeFlowContent()"});
+ }
+
+ function onContentNodeWasRemoved(event)
+ {
+ InspectorTest.assert(event.data.node.getAttribute("id") === "contentRemoved", "\"#contentRemoved\" was removed.");
+ InspectorTest.assert(contentFlow.contentNodes.indexOf(event.data.node) === -1, "\"#contentRemoved\" cannot be found in the contentNodes list.");
+ InspectorTest.completeTest();
+ }
+}
+</script>
+</head>
+<body _onload_="runTest()">
+ <p>Testing that the ContentFlows events are correctly dispatched when content nodes are detached from the DOM.</p>
+
+ <div id="contentStatic" class="content"></div>
+ <div id="contentRemoved" class="content"></div>
+
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (158853 => 158854)
--- trunk/Source/WebCore/ChangeLog 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/Source/WebCore/ChangeLog 2013-11-07 18:05:55 UTC (rev 158854)
@@ -1,3 +1,18 @@
+2013-11-07 Alexandru Chiculita <[email protected]>
+
+ Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+ https://bugs.webkit.org/show_bug.cgi?id=123577
+
+ Reviewed by Timothy Hatcher.
+
+ Test: inspector-protocol/model/content-flow-content-removal.html
+
+ Do not send unregister events for the content nodes of a flow when the element is not part of the DOM
+ anymore. We already send an unbind event, so the inspector is already notified that the node was removed.
+
+ * inspector/InspectorCSSAgent.cpp:
+ (WebCore::InspectorCSSAgent::didUnregisterNamedFlowContentElement):
+
2013-10-30 Jer Noble <[email protected]>
[MSE] Add mock MediaSource classes for testing.
Modified: trunk/Source/WebCore/inspector/InspectorCSSAgent.cpp (158853 => 158854)
--- trunk/Source/WebCore/inspector/InspectorCSSAgent.cpp 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/Source/WebCore/inspector/InspectorCSSAgent.cpp 2013-11-07 18:05:55 UTC (rev 158854)
@@ -775,6 +775,10 @@
ErrorString errorString;
int contentElementNodeId = m_domAgent->pushNodeToFrontend(&errorString, documentNodeId, contentElement);
+ if (!contentElementNodeId) {
+ // We've already notified that the DOM node was removed from the DOM, so there's no need to send another event.
+ return;
+ }
m_frontend->unregisteredNamedFlowContentElement(documentNodeId, namedFlow->name().string(), contentElementNodeId);
}
Modified: trunk/Source/WebInspectorUI/ChangeLog (158853 => 158854)
--- trunk/Source/WebInspectorUI/ChangeLog 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/Source/WebInspectorUI/ChangeLog 2013-11-07 18:05:55 UTC (rev 158854)
@@ -1,3 +1,22 @@
+2013-11-07 Alexandru Chiculita <[email protected]>
+
+ Web Inspector: CSS Regions: Removing a content node of a ContentFlow from the DOM will send a 0 nodeId
+ https://bugs.webkit.org/show_bug.cgi?id=123577
+
+ Reviewed by Timothy Hatcher.
+
+ Fixed the content node removal from the content flow.
+
+ * UserInterface/ContentFlowTreeContentView.js:
+ * UserInterface/DOMTreeManager.js:
+ (WebInspector.DOMTreeManager):
+ (WebInspector.DOMTreeManager.prototype._createContentFlowFromPayload): Registered all the content nodes
+ in the _contentNodesToFlowsMap.
+ (WebInspector.DOMTreeManager.prototype._unbind): Added call to _removeContentNodeFromFlowIfNeeded.
+ (WebInspector.DOMTreeManager.prototype._removeContentNodeFromFlowIfNeeded): Called from _unbind to check
+ and remove a node from it's parent content flow if needed.
+ (WebInspector.DOMTreeManager.prototype.unregisteredNamedFlowContentElement):
+
2013-11-06 Timothy Hatcher <[email protected]>
Properly null check positionToReveal in ResourceSidebarPanel.prototype.showSourceCode.
Modified: trunk/Source/WebInspectorUI/UserInterface/ContentFlowTreeContentView.js (158853 => 158854)
--- trunk/Source/WebInspectorUI/UserInterface/ContentFlowTreeContentView.js 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/Source/WebInspectorUI/UserInterface/ContentFlowTreeContentView.js 2013-11-07 18:05:55 UTC (rev 158854)
@@ -183,6 +183,6 @@
contentNodeOutline.close();
contentNodeOutline.element.remove();
- this._nodesMap.remove(event.data.node.id);
+ this._nodesMap.delete(event.data.node.id);
}
};
Modified: trunk/Source/WebInspectorUI/UserInterface/DOMTreeManager.js (158853 => 158854)
--- trunk/Source/WebInspectorUI/UserInterface/DOMTreeManager.js 2013-11-07 17:58:57 UTC (rev 158853)
+++ trunk/Source/WebInspectorUI/UserInterface/DOMTreeManager.js 2013-11-07 18:05:55 UTC (rev 158854)
@@ -40,6 +40,7 @@
this._document = null;
this._attributeLoadNodeIds = {};
this._flows = {};
+ this._contentNodesToFlowsMap = new Map;
}
WebInspector.Object.addConstructorFunctions(WebInspector.DOMTreeManager);
@@ -337,6 +338,8 @@
*/
_unbind: function(node)
{
+ this._removeContentNodeFromFlowIfNeeded(node);
+
delete this._idToDOMNode[node.id];
for (var i = 0; node.children && i < node.children.length; ++i)
this._unbind(node.children[i]);
@@ -536,7 +539,14 @@
_createContentFlowFromPayload: function(flowPayload)
{
// FIXME: Collect the regions from the payload.
- return new WebInspector.ContentFlow(flowPayload.documentNodeId, flowPayload.name, flowPayload.overset, flowPayload.content.map(this.nodeForId.bind(this)));
+ var flow = new WebInspector.ContentFlow(flowPayload.documentNodeId, flowPayload.name, flowPayload.overset, flowPayload.content.map(this.nodeForId.bind(this)));
+
+ for (var contentNode of flow.contentNodes) {
+ console.assert(!this._contentNodesToFlowsMap.has(contentNode.id));
+ this._contentNodesToFlowsMap.set(contentNode.id, flow);
+ }
+
+ return flow;
},
_updateContentFlowFromPayload: function(contentFlow, flowPayload)
@@ -557,6 +567,7 @@
console.error("Error while getting the named flows for document " + documentNodeIdentifier + ": " + error);
return;
}
+ this._contentNodesToFlowsMap.clear();
var contentFlows = [];
for (var i = 0; i < flows.length; ++i) {
var flowPayload = flows[i];
@@ -614,21 +625,37 @@
{
var flowKey = WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName});
console.assert(this._flows.hasOwnProperty(flowKey));
+ console.assert(!this._contentNodesToFlowsMap.has(contentNodeId));
var flow = this._flows[flowKey];
var contentNode = this.nodeForId(contentNodeId);
+ this._contentNodesToFlowsMap.set(contentNode.id, flow);
+
if (nextContentElementNodeId)
flow.insertContentNodeBefore(contentNode, this.nodeForId(nextContentElementNodeId));
else
flow.appendContentNode(contentNode);
},
+ _removeContentNodeFromFlowIfNeeded: function(node)
+ {
+ if (!this._contentNodesToFlowsMap.has(node.id))
+ return;
+ var flow = this._contentNodesToFlowsMap.get(node.id);
+ this._contentNodesToFlowsMap.delete(node.id);
+ flow.removeContentNode(node);
+ },
+
unregisteredNamedFlowContentElement: function(documentNodeIdentifier, flowName, contentNodeId)
{
- var flowKey = WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName});
- console.assert(this._flows.hasOwnProperty(flowKey));
- this._flows[flowKey].removeContentNode(this.nodeForId(contentNodeId));
+ console.assert(this._contentNodesToFlowsMap.has(contentNodeId));
+
+ var flow = this._contentNodesToFlowsMap.get(contentNodeId);
+ console.assert(flow.id === WebInspector.DOMTreeManager._flowPayloadHashKey({documentNodeId: documentNodeIdentifier, name: flowName}));
+
+ this._contentNodesToFlowsMap.delete(contentNodeId);
+ flow.removeContentNode(this.nodeForId(contentNodeId));
}
}
