Title: [186493] releases/WebKitGTK/webkit-2.8
Revision
186493
Author
carlo...@webkit.org
Date
2015-07-08 01:42:26 -0700 (Wed, 08 Jul 2015)

Log Message

Merge r186461 - REGRESSION(r183706): HTMLImageElement sometimes fails to register as document named item.
<https://webkit.org/b/146679>
<rdar://problem/21613839>

Reviewed by Antti Koivisto.

Source/WebCore:

After r183706, Element::hasName() no longer returns outdated information when called
inside a parseAttribute() override. HTMLImageElement was relying on this to check
if it *used* to have a name attribute before the currently parsing one was set.

Since parseAttribute() only shows subclasses the new attribute value, I'm adding a
flag to HTMLImageElement that remembers whether we had a name attribute or not.

Test: fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html

* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::parseAttribute):
* html/HTMLImageElement.h:

LayoutTests:

Add a test that would assert when removing a named HTMLImageElement from the DOM
after having failed to register it as a document named item.

* fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash-expected.txt: Added.
* fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog (186492 => 186493)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-07-08 06:27:07 UTC (rev 186492)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-07-08 08:42:26 UTC (rev 186493)
@@ -1,3 +1,17 @@
+2015-07-07  Andreas Kling  <akl...@apple.com>
+
+        REGRESSION(r183706): HTMLImageElement sometimes fails to register as document named item.
+        <https://webkit.org/b/146679>
+        <rdar://problem/21613839>
+
+        Reviewed by Antti Koivisto.
+
+        Add a test that would assert when removing a named HTMLImageElement from the DOM
+        after having failed to register it as a document named item.
+
+        * fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash-expected.txt: Added.
+        * fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html: Added.
+
 2015-07-06  Andreas Kling  <akl...@apple.com>
 
         Crash when setting text direction via MakeTextWritingDirection* editing commands.

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash-expected.txt (0 => 186493)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash-expected.txt	2015-07-08 08:42:26 UTC (rev 186493)
@@ -0,0 +1,15 @@
+This test verifies that removing a named HTMLImageElement from the document unregisters its mapping cleanly. It passes if it doesn't assert.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Looking up image element through document named item collection...
+PASS document.nameAttributeValue is imageElement
+Removing it from the document...
+PASS imageElement.parentNode is null
+Checking that named item mapping is gone...
+PASS document.nameAttributeValue is undefined
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html (0 => 186493)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html	2015-07-08 08:42:26 UTC (rev 186493)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<img id="foo">
+<script>
+
+description("This test verifies that removing a named HTMLImageElement from the document unregisters its mapping cleanly. It passes if it doesn't assert.");
+
+var imageElement = document.getElementById("foo");
+imageElement.setAttribute("name", "nameAttributeValue");
+
+debug("Looking up image element through document named item collection...");
+shouldBe("document.nameAttributeValue", "imageElement");
+
+debug("Removing it from the document...");
+imageElement.parentNode.removeChild(imageElement);
+shouldBe("imageElement.parentNode", "null");
+
+debug("Checking that named item mapping is gone...");
+shouldBe("document.nameAttributeValue", "undefined");
+
+</script>
+<script src=""
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (186492 => 186493)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-07-08 06:27:07 UTC (rev 186492)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-07-08 08:42:26 UTC (rev 186493)
@@ -1,3 +1,24 @@
+2015-07-07  Andreas Kling  <akl...@apple.com>
+
+        REGRESSION(r183706): HTMLImageElement sometimes fails to register as document named item.
+        <https://webkit.org/b/146679>
+        <rdar://problem/21613839>
+
+        Reviewed by Antti Koivisto.
+
+        After r183706, Element::hasName() no longer returns outdated information when called
+        inside a parseAttribute() override. HTMLImageElement was relying on this to check
+        if it *used* to have a name attribute before the currently parsing one was set.
+
+        Since parseAttribute() only shows subclasses the new attribute value, I'm adding a
+        flag to HTMLImageElement that remembers whether we had a name attribute or not.
+
+        Test: fast/dom/HTMLImageElement/remove-img-with-name-from-document-crash.html
+
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::parseAttribute):
+        * html/HTMLImageElement.h:
+
 2015-05-28  Carlos Alberto Lopez Perez  <clo...@igalia.com>
 
         [CMake] Improve detection and usage of GL/GLES/EGL libraries.

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.cpp (186492 => 186493)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.cpp	2015-07-08 06:27:07 UTC (rev 186492)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.cpp	2015-07-08 08:42:26 UTC (rev 186493)
@@ -177,7 +177,7 @@
     } else {
         if (name == nameAttr) {
             bool willHaveName = !value.isNull();
-            if (hasName() != willHaveName && inDocument() && is<HTMLDocument>(document())) {
+            if (m_hadNameBeforeAttributeChanged != willHaveName && inDocument() && is<HTMLDocument>(document())) {
                 HTMLDocument& document = downcast<HTMLDocument>(this->document());
                 const AtomicString& id = getIdAttribute();
                 if (!id.isEmpty() && id != getNameAttribute()) {
@@ -187,6 +187,7 @@
                         document.removeDocumentNamedItem(*id.impl(), *this);
                 }
             }
+            m_hadNameBeforeAttributeChanged = willHaveName;
         }
         HTMLElement::parseAttribute(name, value);
     }

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.h (186492 => 186493)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.h	2015-07-08 06:27:07 UTC (rev 186492)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/html/HTMLImageElement.h	2015-07-08 08:42:26 UTC (rev 186493)
@@ -133,6 +133,7 @@
     AtomicString m_lowercasedUsemap;
     float m_imageDevicePixelRatio;
     bool m_experimentalImageMenuEnabled;
+    bool m_hadNameBeforeAttributeChanged { false }; // FIXME: We only need this because parseAttribute() can't see the old value.
 
 #if ENABLE(SERVICE_CONTROLS)
     void updateImageControls();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to