Changeset 223801 in webkit

Timestamp:

Oct 20, 2017 5:03:18 PM (7 years ago)

Author:

rniwa@webkit.org

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLSourceElement.cpp (modified) (2 diffs)
• html/HTMLTrackElement.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/html/HTMLSourceElement.cpp

@@ -67 +67 @@
 {
     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))
@@ -81 +81 @@
 
 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);
 }
 

trunk/Source/WebCore/html/HTMLTrackElement.cpp

@@ -81 +81 @@
     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;
@@ -96 +93 @@
     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);
 }