Changeset 242241 in webkit

Timestamp:

Feb 28, 2019 4:48:01 PM (5 years ago)

Author:

Joseph Pecoraro

Message:

Web Inspector: View.removeSubview not removing the element properly when not parented
​https://bugs.webkit.org/show_bug.cgi?id=195146

Reviewed by Matt Baker.

Source/WebInspectorUI:

• UserInterface/Views/View.js:

(WI.View.prototype.removeSubview):
Since the element may not be a direct child, just use Element.prototype.remove.

LayoutTests:

• inspector/view/basics-expected.txt:
• inspector/view/basics.html:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/inspector/view/basics-expected.txt (modified) (1 diff)
• LayoutTests/inspector/view/basics.html (modified) (1 diff)
• Source/WebInspectorUI/ChangeLog (modified) (1 diff)
• Source/WebInspectorUI/UserInterface/Views/View.js (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-02-28  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: View.removeSubview not removing the element properly when not parented
+        https://bugs.webkit.org/show_bug.cgi?id=195146
+
+        Reviewed by Matt Baker.
+
+        * inspector/view/basics-expected.txt:
+        * inspector/view/basics.html:
+
 2019-02-28  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/LayoutTests/inspector/view/basics-expected.txt

@@ -32 +32 @@
 PASS: Removing a nonexistent view should have no effect.
 
+-- Running test case: View.removeSubview.IndirectDescendant
+PASS: Removed view should not be in the DOM.
+
 -- Running test case: View.insertSubviewBefore
 PASS: Inserting a view before `null` should append the view.

trunk/LayoutTests/inspector/view/basics.html

@@ -77 +77 @@
             parent.removeSubview(new WI.View);
             InspectorTest.expectShallowEqual(previousSubviews, parent.subviews, "Removing a nonexistent view should have no effect.")
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "View.removeSubview.IndirectDescendant",
+        test() {
+            let parent = new WI.View;
+            let middleElement = parent.element.appendChild(document.createElement("div"));
+
+            let child = new WI.View;
+            middleElement.appendChild(child.element);
+
+            parent.addSubview(child);
+            parent.removeSubview(child);
+
+            InspectorTest.expectFalse(child.element.parentNode, "Removed view should not be in the DOM.");
 
             return true;

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-02-28  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: View.removeSubview not removing the element properly when not parented
+        https://bugs.webkit.org/show_bug.cgi?id=195146
+
+        Reviewed by Matt Baker.
+
+        * UserInterface/Views/View.js:
+        (WI.View.prototype.removeSubview):
+        Since the element may not be a direct child, just use Element.prototype.remove.
+
 2019-02-28  Matt Baker  <mattbaker@apple.com>
 

trunk/Source/WebInspectorUI/UserInterface/Views/View.js

@@ -109 +109 @@
         this._subviews.insertAtIndex(view, beforeIndex);
 
+        console.assert(!view.element.parentNode || this._element.contains(view.element.parentNode), "Subview DOM element must be a descendant of the parent view element.");
         if (!view.element.parentNode)
             this._element.insertBefore(view.element, referenceView ? referenceView.element : null);
@@ -127 +128 @@
 
         this._subviews.splice(index, 1);
-        this._element.removeChild(view.element);
+        view.element.remove();
 
         view._didMoveToParent(null);