Web Inspector: View.removeSubview not removing the element properly when not parented
authorjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Mar 2019 00:48:01 +0000 (00:48 +0000)
committerjoepeck@webkit.org <joepeck@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Mar 2019 00:48:01 +0000 (00:48 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242241 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/view/basics-expected.txt
LayoutTests/inspector/view/basics.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Views/View.js

index b57feed..c7b4515 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Use-after-move in RenderCombineText::combineTextIfNeeded()
index 2502868..944fcba 100644 (file)
@@ -31,6 +31,9 @@ PASS: Removed view should not be a descendant of the parent.
 PASS: Removed view should not be included in the parent's subviews.
 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.
 PASS: child2 should be inserted before dhild1.
index b277824..68332f5 100644 (file)
@@ -82,6 +82,24 @@ function test()
     });
 
     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;
+        }
+    });
+
+    suite.addTestCase({
         name: "View.insertSubviewBefore",
         test() {
             let parent = new WI.View;
index f01cde2..dfeb033 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         Web Inspector: Debugger: disabled breakpoint color is too dark
index 4f2591f..5494f98 100644 (file)
@@ -108,6 +108,7 @@ WI.View = class View extends WI.Object
 
         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);
 
@@ -126,7 +127,7 @@ WI.View = class View extends WI.Object
         }
 
         this._subviews.splice(index, 1);
-        this._element.removeChild(view.element);
+        view.element.remove();
 
         view._didMoveToParent(null);
     }