Fix conditions in HTMLSourceElement and HTMLTrackElement's insertedIn…

…to and removedFrom

https://bugs.webkit.org/show_bug.cgi?id=178607

Reviewed by Eric Carlson.

Fixed the conditions in insertedInto and removedFrom of HTMLSourceElement and HTMLTrackElement to be
semantically sensisble. Since these elements are only functional when their immediate parents are
HTMLMediaElement and HTMLPictureElement, we have to check that its immediate parent changed, not when
some of its ancestor had changed by insertion or removal.

* html/HTMLSourceElement.cpp:
(WebCore::HTMLSourceElement::insertedInto):
(WebCore::HTMLSourceElement::removedFrom):
* html/HTMLTrackElement.cpp:
(WebCore::HTMLTrackElement::insertedInto):
(WebCore::HTMLTrackElement::removedFrom):


Canonical link: https://commits.webkit.org/194803@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223801 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,22 @@
+2017-10-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Fix conditions in HTMLSourceElement and HTMLTrackElement's insertedInto and removedFrom
+        https://bugs.webkit.org/show_bug.cgi?id=178607
+
+        Reviewed by Eric Carlson.
+
+        Fixed the conditions in insertedInto and removedFrom of HTMLSourceElement and HTMLTrackElement to be
+        semantically sensisble. Since these elements are only functional when their immediate parents are
+        HTMLMediaElement and HTMLPictureElement, we have to check that its immediate parent changed, not when
+        some of its ancestor had changed by insertion or removal.
+
+        * html/HTMLSourceElement.cpp:
+        (WebCore::HTMLSourceElement::insertedInto):
+        (WebCore::HTMLSourceElement::removedFrom):
+        * html/HTMLTrackElement.cpp:
+        (WebCore::HTMLTrackElement::insertedInto):
+        (WebCore::HTMLTrackElement::removedFrom):
+
 2017-10-20  Keith Miller  <keith_miller@apple.com>
 
         Unreviewed, fix windows build.

Source/WebCore/html/HTMLSourceElement.cpp

@@ -66,8 +66,8 @@ Ref<HTMLSourceElement> HTMLSourceElement::create(Document& document)
 Node::InsertedIntoResult HTMLSourceElement::insertedInto(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
 {
     HTMLElement::insertedInto(insertionType, parentOfInsertedTree);
-    // FIXME: This code is wrong if an ancestor of the parent had been inserted.
-    if (auto* parent = parentElement()) {
+    RefPtr<Element> parent = parentElement();
+    if (parent == &parentOfInsertedTree) {
 #if ENABLE(VIDEO)
         if (is<HTMLMediaElement>(*parent))
             downcast<HTMLMediaElement>(*parent).sourceWasAdded(*this);
@@ -80,21 +80,17 @@ Node::InsertedIntoResult HTMLSourceElement::insertedInto(InsertionType insertion
 }
 
 void HTMLSourceElement::removedFrom(RemovalType removalType, ContainerNode& parentOfRemovedTree)
-{
-    // FIXME: This code is wrong if an ancestor of the parent had been removed.
-    RefPtr<Element> parent = parentElement();
-    if (!parent && is<Element>(parentOfRemovedTree))
-        parent = &downcast<Element>(parentOfRemovedTree);
-    if (parent) {
+{        
+    HTMLElement::removedFrom(removalType, parentOfRemovedTree);
+    if (!parentNode() && is<Element>(parentOfRemovedTree)) {
 #if ENABLE(VIDEO)
-        if (is<HTMLMediaElement>(*parent))
-            downcast<HTMLMediaElement>(*parent).sourceWasRemoved(*this);
+        if (is<HTMLMediaElement>(parentOfRemovedTree))
+            downcast<HTMLMediaElement>(parentOfRemovedTree).sourceWasRemoved(*this);
         else
 #endif
-        if (is<HTMLPictureElement>(*parent))
-            downcast<HTMLPictureElement>(*parent).sourcesChanged();
+        if (is<HTMLPictureElement>(parentOfRemovedTree))
+            downcast<HTMLPictureElement>(parentOfRemovedTree).sourcesChanged();
     }
-    HTMLElement::removedFrom(removalType, parentOfRemovedTree);
 }
 
 void HTMLSourceElement::scheduleErrorEvent()

Source/WebCore/html/HTMLTrackElement.cpp

@@ -80,13 +80,10 @@ Node::InsertedIntoResult HTMLTrackElement::insertedInto(InsertionType insertionT
 {
     HTMLElement::insertedInto(insertionType, parentOfInsertedTree);
 
-    // FIXME: This code is wrong. If HTMLTrackElement can be any descendent of HTMLMediaElement, then check ancestors of parentOfInsertedTree.
-    // If HTMLMediaElement only supports HTMLTrackElement which is an immediate child, then check parentNode() instead.
-    if (is<HTMLMediaElement>(parentOfInsertedTree))
+    if (parentNode() == &parentOfInsertedTree && is<HTMLMediaElement>(parentOfInsertedTree)) {
         downcast<HTMLMediaElement>(parentOfInsertedTree).didAddTextTrack(*this);
-
-    // Since we've moved to a new parent, we may now be able to load.
-    scheduleLoad();
+        scheduleLoad();
+    }
 
     return InsertedIntoResult::Done;
 }
@@ -95,9 +92,7 @@ void HTMLTrackElement::removedFrom(RemovalType removalType, ContainerNode& paren
 {
     HTMLElement::removedFrom(removalType, parentOfRemovedTree);
 
-    // FIXME: This code is wrong. If HTMLTrackElement can be any descendent of HTMLMediaElement, then check ancestors of parentOfInsertedTree.
-    // If HTMLMediaElement only supports HTMLTrackElement which is an immediate child, then check parentNode() instead.
-    if (is<HTMLMediaElement>(parentOfRemovedTree))
+    if (!parentNode() && is<HTMLMediaElement>(parentOfRemovedTree))
         downcast<HTMLMediaElement>(parentOfRemovedTree).didRemoveTextTrack(*this);
 }