Title: [189804] releases/WebKitGTK/webkit-2.10
Revision
189804
Author
carlo...@webkit.org
Date
2015-09-15 01:57:56 -0700 (Tue, 15 Sep 2015)

Log Message

Merge r189680 - Document.title does not behave according to specification
https://bugs.webkit.org/show_bug.cgi?id=149098

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Rebaseline several W3C tests now that more checks are passing.

* web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-01-expected.txt:
* web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-02-expected.txt:

Source/WebCore:

Update Document.title to behave according to the latest DOM specification:
https://html.spec.whatwg.org/multipage/dom.html#document.title

In particular, the following Web-Exposed changes were made:
1. The title Element should be the first title element in the document
   (in tree order) [1]. Previously, WebKit would use the first title
   Element *added* to the Document. Document.title returns the text
   content of the title Element so this change is web-exposed.
2. If the title Element is replaced after the title has been set by the
   JS (via the document.title setter), we should update the value
   returned by the document.title getter. Previously, WebKit would set
   a flag if the title was explicitly set by JS via document.title
   setter and later title element changes would not override the title
   set by the JS. This behavior isn't specified and does not match the
   behavior of other browsers.

The new behavior is also consistent with the behavior of Firefox and
Chrome.

Some refactoring was made for the sake of clarity now that our
implementation has changed. See details below.

[1] https://html.spec.whatwg.org/multipage/dom.html#the-title-element-2

No new tests, already covered by existing tests.

* dom/Document.cpp:
(WebCore::Document::updateTitleFromTitleElement):
New convenience method that calls updateTitle() with the text of the
document's current title Element. If there is no title Element, it
clears the title.

(WebCore::Document::updateTitleElement):
Method which updates the Document's title Element whenever a title
Element is added or removed from the Document. Once the title Element
is updated, it takes care of calling updateTitleFromTitleElement() to
update the Document's title.

(WebCore::Document::titleElementAdded):
(WebCore::Document::titleElementRemoved):
(WebCore::Document::titleElementTextChanged):
New Document public API called by HTMLTitleElement / SVGTitleElement
whenever a title Element is added / removed from the Document or
whenever the title element's text has changed. These methods will
take care of calling updateTitleElement() / updateTitleFromTitleElement()
as necessary.
Previously, we would only have 2 methods:
- setTitleElement() which would be called whenever a title Element was
  added to the document or when its text had changed. The name was
  confusing because it would not necessarily set the document's title
  Element and it would be used both for title element update and a
  simple title update. This method has been split into 2:
  titleElementAdded() and titleElementTextChanged().
- removeTitle() which would be called whenever a title Element was
  removed. The naming was confusing because it would not necessarily
  remove the Document's title Element. This is now called
  titleElementRemoved().

* html/HTMLTitleElement.cpp:
(WebCore::HTMLTitleElement::insertedInto):
Call the new titleElementAdded() instead of setTitleElement().

(WebCore::HTMLTitleElement::removedFrom):
Call the new titleElementRemoved() instead of removeTitle().

(WebCore::HTMLTitleElement::childrenChanged):
Call the new titleElementTextChanged() instead of
setTitleElement() / removeTitle() as we don't really want
to remove or add a title Element. We merely want to notify
the document that the title element text has changed in
case it is the current title Element of the Document.

(WebCore::HTMLTitleElement::computedTextWithDirection):
Rename textWithDirection() to computedTextWithDirection() to
make it clear it is not a simple getter and make it private
as it is only used to set the m_title member which caches the
computed text.

* html/HTMLTitleElement.h:
Add new textWithDirection() getter which returns m_title. This
is needed so that Document can query the title of the Element.
Previously, HTMLTitleElement would pass directly m_title to
the Document when calling Document::setTitleElement().

* svg/SVGTitleElement.cpp:
(WebCore::SVGTitleElement::insertedInto):
Call the new titleElementAdded() instead of setTitleElement().

(WebCore::SVGTitleElement::removedFrom):
Call the new titleElementRemoved() instead of removeTitle().

(WebCore::SVGTitleElement::childrenChanged):
Call the new titleElementTextChanged() instead of
setTitleElement().

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.10/LayoutTests/imported/w3c/ChangeLog (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/imported/w3c/ChangeLog	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/imported/w3c/ChangeLog	2015-09-15 08:57:56 UTC (rev 189804)
@@ -1,5 +1,17 @@
 2015-09-13  Chris Dumez  <cdu...@apple.com>
 
+        Document.title does not behave according to specification
+        https://bugs.webkit.org/show_bug.cgi?id=149098
+
+        Reviewed by Ryosuke Niwa.
+
+        Rebaseline several W3C tests now that more checks are passing.
+
+        * web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-01-expected.txt:
+        * web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-02-expected.txt:
+
+2015-09-13  Chris Dumez  <cdu...@apple.com>
+
         document.lastModified should use the user's local time zone
         https://bugs.webkit.org/show_bug.cgi?id=149092
         <rdar://problem/22567705>

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2015-09-15 08:57:56 UTC (rev 189804)
@@ -1,5 +1,107 @@
 2015-09-13  Chris Dumez  <cdu...@apple.com>
 
+        Document.title does not behave according to specification
+        https://bugs.webkit.org/show_bug.cgi?id=149098
+
+        Reviewed by Ryosuke Niwa.
+
+        Update Document.title to behave according to the latest DOM specification:
+        https://html.spec.whatwg.org/multipage/dom.html#document.title
+
+        In particular, the following Web-Exposed changes were made:
+        1. The title Element should be the first title element in the document
+           (in tree order) [1]. Previously, WebKit would use the first title
+           Element *added* to the Document. Document.title returns the text
+           content of the title Element so this change is web-exposed.
+        2. If the title Element is replaced after the title has been set by the
+           JS (via the document.title setter), we should update the value
+           returned by the document.title getter. Previously, WebKit would set
+           a flag if the title was explicitly set by JS via document.title
+           setter and later title element changes would not override the title
+           set by the JS. This behavior isn't specified and does not match the
+           behavior of other browsers.
+
+        The new behavior is also consistent with the behavior of Firefox and
+        Chrome.
+
+        Some refactoring was made for the sake of clarity now that our
+        implementation has changed. See details below.
+
+        [1] https://html.spec.whatwg.org/multipage/dom.html#the-title-element-2
+
+        No new tests, already covered by existing tests.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateTitleFromTitleElement):
+        New convenience method that calls updateTitle() with the text of the
+        document's current title Element. If there is no title Element, it
+        clears the title.
+
+        (WebCore::Document::updateTitleElement):
+        Method which updates the Document's title Element whenever a title
+        Element is added or removed from the Document. Once the title Element
+        is updated, it takes care of calling updateTitleFromTitleElement() to
+        update the Document's title.
+
+        (WebCore::Document::titleElementAdded):
+        (WebCore::Document::titleElementRemoved):
+        (WebCore::Document::titleElementTextChanged):
+        New Document public API called by HTMLTitleElement / SVGTitleElement
+        whenever a title Element is added / removed from the Document or
+        whenever the title element's text has changed. These methods will
+        take care of calling updateTitleElement() / updateTitleFromTitleElement()
+        as necessary.
+        Previously, we would only have 2 methods:
+        - setTitleElement() which would be called whenever a title Element was
+          added to the document or when its text had changed. The name was
+          confusing because it would not necessarily set the document's title
+          Element and it would be used both for title element update and a
+          simple title update. This method has been split into 2:
+          titleElementAdded() and titleElementTextChanged().
+        - removeTitle() which would be called whenever a title Element was
+          removed. The naming was confusing because it would not necessarily
+          remove the Document's title Element. This is now called
+          titleElementRemoved().
+
+        * html/HTMLTitleElement.cpp:
+        (WebCore::HTMLTitleElement::insertedInto):
+        Call the new titleElementAdded() instead of setTitleElement().
+
+        (WebCore::HTMLTitleElement::removedFrom):
+        Call the new titleElementRemoved() instead of removeTitle().
+
+        (WebCore::HTMLTitleElement::childrenChanged):
+        Call the new titleElementTextChanged() instead of
+        setTitleElement() / removeTitle() as we don't really want
+        to remove or add a title Element. We merely want to notify
+        the document that the title element text has changed in
+        case it is the current title Element of the Document.
+
+        (WebCore::HTMLTitleElement::computedTextWithDirection):
+        Rename textWithDirection() to computedTextWithDirection() to
+        make it clear it is not a simple getter and make it private
+        as it is only used to set the m_title member which caches the
+        computed text.
+
+        * html/HTMLTitleElement.h:
+        Add new textWithDirection() getter which returns m_title. This
+        is needed so that Document can query the title of the Element.
+        Previously, HTMLTitleElement would pass directly m_title to
+        the Document when calling Document::setTitleElement().
+
+        * svg/SVGTitleElement.cpp:
+        (WebCore::SVGTitleElement::insertedInto):
+        Call the new titleElementAdded() instead of setTitleElement().
+
+        (WebCore::SVGTitleElement::removedFrom):
+        Call the new titleElementRemoved() instead of removeTitle().
+
+        (WebCore::SVGTitleElement::childrenChanged):
+        Call the new titleElementTextChanged() instead of
+        setTitleElement().
+
+2015-09-13  Chris Dumez  <cdu...@apple.com>
+
         document.lastModified should use the user's local time zone
         https://bugs.webkit.org/show_bug.cgi?id=149092
         <rdar://problem/22567705>

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.cpp (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.cpp	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.cpp	2015-09-15 08:57:56 UTC (rev 189804)
@@ -130,6 +130,7 @@
 #include "SVGElement.h"
 #include "SVGElementFactory.h"
 #include "SVGNames.h"
+#include "SVGTitleElement.h"
 #include "SchemeRegistry.h"
 #include "ScopedEventQueue.h"
 #include "ScriptController.h"
@@ -453,7 +454,6 @@
     , m_frameElementsShouldIgnoreScrolling(false)
     , m_updateFocusAppearanceRestoresSelection(false)
     , m_ignoreDestructiveWriteCount(0)
-    , m_titleSetExplicitly(false)
     , m_markers(std::make_unique<DocumentMarkerController>())
     , m_updateFocusAppearanceTimer(*this, &Document::updateFocusAppearanceTimerFired)
     , m_cssTarget(nullptr)
@@ -1522,10 +1522,23 @@
         loader->setTitle(m_title);
 }
 
+void Document::updateTitleFromTitleElement()
+{
+    if (!m_titleElement) {
+        updateTitle(StringWithDirection());
+        return;
+    }
+
+    if (is<HTMLTitleElement>(*m_titleElement))
+        updateTitle(downcast<HTMLTitleElement>(*m_titleElement).textWithDirection());
+    else if (is<SVGTitleElement>(*m_titleElement)) {
+        // FIXME: does SVG have a title text direction?
+        updateTitle(StringWithDirection(downcast<SVGTitleElement>(*m_titleElement).textContent(), LTR));
+    }
+}
+
 void Document::setTitle(const String& title)
 {
-    // Title set by _javascript_ -- overrides any title elements.
-    m_titleSetExplicitly = true;
     if (!isHTMLDocument() && !isXHTMLDocument())
         m_titleElement = nullptr;
     else if (!m_titleElement) {
@@ -1543,37 +1556,44 @@
         downcast<HTMLTitleElement>(*m_titleElement).setText(title);
 }
 
-void Document::setTitleElement(const StringWithDirection& title, Element* titleElement)
+void Document::updateTitleElement(Element* newTitleElement)
 {
-    if (titleElement != m_titleElement) {
-        if (m_titleElement || m_titleSetExplicitly) {
-            // Only allow the first title element to change the title -- others have no effect.
-            return;
-        }
-        m_titleElement = titleElement;
-    }
+    // Only allow the first title element in tree order to change the title -- others have no effect.
+    if (m_titleElement) {
+        if (isHTMLDocument() || isXHTMLDocument())
+            m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first();
+        else if (isSVGDocument())
+            m_titleElement = descendantsOfType<SVGTitleElement>(*this).first();
+    } else
+        m_titleElement = newTitleElement;
 
-    updateTitle(title);
+    updateTitleFromTitleElement();
 }
 
-void Document::removeTitle(Element* titleElement)
+void Document::titleElementAdded(Element& titleElement)
 {
-    if (m_titleElement != titleElement)
+    if (m_titleElement == &titleElement)
         return;
 
-    m_titleElement = nullptr;
-    m_titleSetExplicitly = false;
+    updateTitleElement(&titleElement);
+}
 
-    // Update title based on first title element in the head, if one exists.
-    if (HTMLElement* headElement = head()) {
-        if (auto firstTitle = childrenOfType<HTMLTitleElement>(*headElement).first())
-            setTitleElement(firstTitle->textWithDirection(), firstTitle);
-    }
+void Document::titleElementRemoved(Element& titleElement)
+{
+    if (m_titleElement != &titleElement)
+        return;
 
-    if (!m_titleElement)
-        updateTitle(StringWithDirection());
+    updateTitleElement(nullptr);
 }
 
+void Document::titleElementTextChanged(Element& titleElement)
+{
+    if (m_titleElement != &titleElement)
+        return;
+
+    updateTitleFromTitleElement();
+}
+
 void Document::registerForVisibilityStateChangedCallbacks(Element* element)
 {
     m_visibilityStateCallbackElements.add(element);

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.h (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.h	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.h	2015-09-15 08:57:56 UTC (rev 189804)
@@ -836,8 +836,9 @@
     String title() const { return m_title.string(); }
     void setTitle(const String&);
 
-    void setTitleElement(const StringWithDirection&, Element* titleElement);
-    void removeTitle(Element* titleElement);
+    void titleElementAdded(Element& titleElement);
+    void titleElementRemoved(Element& titleElement);
+    void titleElementTextChanged(Element& titleElement);
 
     String cookie(ExceptionCode&);
     void setCookie(const String&, ExceptionCode&);
@@ -1291,6 +1292,8 @@
     friend class Node;
     friend class IgnoreDestructiveWriteCountIncrementer;
 
+    void updateTitleElement(Element* newTitleElement);
+
     void commonTeardown();
 
     RenderObject* renderer() const = delete;
@@ -1324,6 +1327,7 @@
 
     virtual double timerAlignmentInterval(bool hasReachedMaxNestingLevel) const override final;
 
+    void updateTitleFromTitleElement();
     void updateTitle(const StringWithDirection&);
     void updateFocusAppearanceTimerFired();
     void updateBaseURL();
@@ -1479,7 +1483,6 @@
 
     StringWithDirection m_title;
     StringWithDirection m_rawTitle;
-    bool m_titleSetExplicitly;
     RefPtr<Element> m_titleElement;
 
     std::unique_ptr<AXObjectCache> m_axObjectCache;

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.cpp (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.cpp	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.cpp	2015-09-15 08:57:56 UTC (rev 189804)
@@ -53,7 +53,7 @@
 {
     HTMLElement::insertedInto(insertionPoint);
     if (inDocument() && !isInShadowTree())
-        document().setTitleElement(m_title, this);
+        document().titleElementAdded(*this);
     return InsertionDone;
 }
 
@@ -61,19 +61,14 @@
 {
     HTMLElement::removedFrom(insertionPoint);
     if (insertionPoint.inDocument() && !insertionPoint.isInShadowTree())
-        document().removeTitle(this);
+        document().titleElementRemoved(*this);
 }
 
 void HTMLTitleElement::childrenChanged(const ChildChange& change)
 {
     HTMLElement::childrenChanged(change);
-    m_title = textWithDirection();
-    if (inDocument()) {
-        if (!isInShadowTree())
-            document().setTitleElement(m_title, this);
-        else
-            document().removeTitle(this);
-    }
+    m_title = computedTextWithDirection();
+    document().titleElementTextChanged(*this);
 }
 
 String HTMLTitleElement::text() const
@@ -81,7 +76,7 @@
     return TextNodeTraversal::contentsAsString(*this);
 }
 
-StringWithDirection HTMLTitleElement::textWithDirection()
+StringWithDirection HTMLTitleElement::computedTextWithDirection()
 {
     TextDirection direction = LTR;
     if (RenderStyle* computedStyle = this->computedStyle())

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.h (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.h	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.h	2015-09-15 08:57:56 UTC (rev 189804)
@@ -34,7 +34,7 @@
     String text() const;
     void setText(const String&);
 
-    StringWithDirection textWithDirection();
+    const StringWithDirection& textWithDirection() const { return m_title; }
 
 private:
     HTMLTitleElement(const QualifiedName&, Document&);
@@ -43,6 +43,8 @@
     virtual void removedFrom(ContainerNode&) override;
     virtual void childrenChanged(const ChildChange&) override;
 
+    StringWithDirection computedTextWithDirection();
+
     StringWithDirection m_title;
 };
 

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/svg/SVGTitleElement.cpp (189803 => 189804)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/svg/SVGTitleElement.cpp	2015-09-15 08:39:09 UTC (rev 189803)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/svg/SVGTitleElement.cpp	2015-09-15 08:57:56 UTC (rev 189804)
@@ -43,10 +43,8 @@
     if (!rootParent.inDocument())
         return InsertionDone;
 
-    if (firstChild() && document().isSVGDocument()) {
-        // FIXME: does SVG have a title text direction?
-        document().setTitleElement(StringWithDirection(textContent(), LTR), this);
-    }
+    if (firstChild() && document().isSVGDocument())
+        document().titleElementAdded(*this);
     return InsertionDone;
 }
 
@@ -54,16 +52,13 @@
 {
     SVGElement::removedFrom(rootParent);
     if (rootParent.inDocument() && document().isSVGDocument())
-        document().removeTitle(this);
+        document().titleElementRemoved(*this);
 }
 
 void SVGTitleElement::childrenChanged(const ChildChange& change)
 {
     SVGElement::childrenChanged(change);
-    if (inDocument() && document().isSVGDocument()) {
-        // FIXME: does SVG have title text direction?
-        document().setTitleElement(StringWithDirection(textContent(), LTR), this);
-    }
+    document().titleElementTextChanged(*this);
 }
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to