Changeset 189680 in webkit


Ignore:
Timestamp:
Sep 13, 2015 10:03:29 PM (9 years ago)
Author:
Chris Dumez
Message:

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().

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r189679 r189680  
     12015-09-13  Chris Dumez  <cdumez@apple.com>
     2
     3        Document.title does not behave according to specification
     4        https://bugs.webkit.org/show_bug.cgi?id=149098
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Rebaseline several W3C tests now that more checks are passing.
     9
     10        * web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-01-expected.txt:
     11        * web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-02-expected.txt:
     12
    1132015-09-13  Chris Dumez  <cdumez@apple.com>
    214

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-01-expected.txt

    r189555 r189680  
    22PASS document.title with head blown away
    33PASS document.title with head blown away 1
    4 FAIL Untitled 2 assert_equals: expected "PASS" but got ""
    5 FAIL PASS 3 assert_equals: expected "PASS2" but got ""
     4PASS Untitled 2
     5PASS PASS 3
    66

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/dom/documents/dom-tree-accessors/document.title-02-expected.txt

    r189555 r189680  
    22PASS document.title with head blown away
    33PASS document.title with head blown away 1
    4 FAIL Untitled 2 assert_equals: expected "PASS" but got ""
    5 FAIL PASS 3 assert_equals: expected "PASS2" but got ""
     4PASS Untitled 2
     5PASS PASS 3
    66

  • trunk/Source/WebCore/ChangeLog

    r189679 r189680  
     12015-09-13  Chris Dumez  <cdumez@apple.com>
     2
     3        Document.title does not behave according to specification
     4        https://bugs.webkit.org/show_bug.cgi?id=149098
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Update Document.title to behave according to the latest DOM specification:
     9        https://html.spec.whatwg.org/multipage/dom.html#document.title
     10
     11        In particular, the following Web-Exposed changes were made:
     12        1. The title Element should be the first title element in the document
     13           (in tree order) [1]. Previously, WebKit would use the first title
     14           Element *added* to the Document. Document.title returns the text
     15           content of the title Element so this change is web-exposed.
     16        2. If the title Element is replaced after the title has been set by the
     17           JS (via the document.title setter), we should update the value
     18           returned by the document.title getter. Previously, WebKit would set
     19           a flag if the title was explicitly set by JS via document.title
     20           setter and later title element changes would not override the title
     21           set by the JS. This behavior isn't specified and does not match the
     22           behavior of other browsers.
     23
     24        The new behavior is also consistent with the behavior of Firefox and
     25        Chrome.
     26
     27        Some refactoring was made for the sake of clarity now that our
     28        implementation has changed. See details below.
     29
     30        [1] https://html.spec.whatwg.org/multipage/dom.html#the-title-element-2
     31
     32        No new tests, already covered by existing tests.
     33
     34        * dom/Document.cpp:
     35        (WebCore::Document::updateTitleFromTitleElement):
     36        New convenience method that calls updateTitle() with the text of the
     37        document's current title Element. If there is no title Element, it
     38        clears the title.
     39
     40        (WebCore::Document::updateTitleElement):
     41        Method which updates the Document's title Element whenever a title
     42        Element is added or removed from the Document. Once the title Element
     43        is updated, it takes care of calling updateTitleFromTitleElement() to
     44        update the Document's title.
     45
     46        (WebCore::Document::titleElementAdded):
     47        (WebCore::Document::titleElementRemoved):
     48        (WebCore::Document::titleElementTextChanged):
     49        New Document public API called by HTMLTitleElement / SVGTitleElement
     50        whenever a title Element is added / removed from the Document or
     51        whenever the title element's text has changed. These methods will
     52        take care of calling updateTitleElement() / updateTitleFromTitleElement()
     53        as necessary.
     54        Previously, we would only have 2 methods:
     55        - setTitleElement() which would be called whenever a title Element was
     56          added to the document or when its text had changed. The name was
     57          confusing because it would not necessarily set the document's title
     58          Element and it would be used both for title element update and a
     59          simple title update. This method has been split into 2:
     60          titleElementAdded() and titleElementTextChanged().
     61        - removeTitle() which would be called whenever a title Element was
     62          removed. The naming was confusing because it would not necessarily
     63          remove the Document's title Element. This is now called
     64          titleElementRemoved().
     65
     66        * html/HTMLTitleElement.cpp:
     67        (WebCore::HTMLTitleElement::insertedInto):
     68        Call the new titleElementAdded() instead of setTitleElement().
     69
     70        (WebCore::HTMLTitleElement::removedFrom):
     71        Call the new titleElementRemoved() instead of removeTitle().
     72
     73        (WebCore::HTMLTitleElement::childrenChanged):
     74        Call the new titleElementTextChanged() instead of
     75        setTitleElement() / removeTitle() as we don't really want
     76        to remove or add a title Element. We merely want to notify
     77        the document that the title element text has changed in
     78        case it is the current title Element of the Document.
     79
     80        (WebCore::HTMLTitleElement::computedTextWithDirection):
     81        Rename textWithDirection() to computedTextWithDirection() to
     82        make it clear it is not a simple getter and make it private
     83        as it is only used to set the m_title member which caches the
     84        computed text.
     85
     86        * html/HTMLTitleElement.h:
     87        Add new textWithDirection() getter which returns m_title. This
     88        is needed so that Document can query the title of the Element.
     89        Previously, HTMLTitleElement would pass directly m_title to
     90        the Document when calling Document::setTitleElement().
     91
     92        * svg/SVGTitleElement.cpp:
     93        (WebCore::SVGTitleElement::insertedInto):
     94        Call the new titleElementAdded() instead of setTitleElement().
     95
     96        (WebCore::SVGTitleElement::removedFrom):
     97        Call the new titleElementRemoved() instead of removeTitle().
     98
     99        (WebCore::SVGTitleElement::childrenChanged):
     100        Call the new titleElementTextChanged() instead of
     101        setTitleElement().
     102
    11032015-09-13  Chris Dumez  <cdumez@apple.com>
    2104

  • trunk/Source/WebCore/dom/Document.cpp

    r189679 r189680  
    130130#include "SVGElementFactory.h"
    131131#include "SVGNames.h"
     132#include "SVGTitleElement.h"
    132133#include "SchemeRegistry.h"
    133134#include "ScopedEventQueue.h"
    … …  
    454455    , m_updateFocusAppearanceRestoresSelection(false)
    455456    , m_ignoreDestructiveWriteCount(0)
    456     , m_titleSetExplicitly(false)
    457457    , m_markers(std::make_unique<DocumentMarkerController>())
    458458    , m_updateFocusAppearanceTimer(*this, &Document::updateFocusAppearanceTimerFired)
    … …  
    15331533}
    15341534
     1535void Document::updateTitleFromTitleElement()
     1536{
     1537    if (!m_titleElement) {
     1538        updateTitle(StringWithDirection());
     1539        return;
     1540    }
     1541
     1542    if (is<HTMLTitleElement>(*m_titleElement))
     1543        updateTitle(downcast<HTMLTitleElement>(*m_titleElement).textWithDirection());
     1544    else if (is<SVGTitleElement>(*m_titleElement)) {
     1545        // FIXME: does SVG have a title text direction?
     1546        updateTitle(StringWithDirection(downcast<SVGTitleElement>(*m_titleElement).textContent(), LTR));
     1547    }
     1548}
     1549
    15351550void Document::setTitle(const String& title)
    15361551{
    1537     // Title set by JavaScript -- overrides any title elements.
    1538     m_titleSetExplicitly = true;
    15391552    if (!isHTMLDocument() && !isXHTMLDocument())
    15401553        m_titleElement = nullptr;
    … …  
    15541567}
    15551568
    1556 void Document::setTitleElement(const StringWithDirection& title, Element* titleElement)
    1557 {
    1558     if (titleElement != m_titleElement) {
    1559         if (m_titleElement || m_titleSetExplicitly) {
    1560             // Only allow the first title element to change the title -- others have no effect.
    1561             return;
    1562         }
    1563         m_titleElement = titleElement;
    1564     }
    1565 
    1566     updateTitle(title);
    1567 }
    1568 
    1569 void Document::removeTitle(Element* titleElement)
    1570 {
    1571     if (m_titleElement != titleElement)
    1572         return;
    1573 
    1574     m_titleElement = nullptr;
    1575     m_titleSetExplicitly = false;
    1576 
    1577     // Update title based on first title element in the head, if one exists.
    1578     if (HTMLElement* headElement = head()) {
    1579         if (auto firstTitle = childrenOfType<HTMLTitleElement>(*headElement).first())
    1580             setTitleElement(firstTitle->textWithDirection(), firstTitle);
    1581     }
    1582 
    1583     if (!m_titleElement)
    1584         updateTitle(StringWithDirection());
     1569void Document::updateTitleElement(Element* newTitleElement)
     1570{
     1571    // Only allow the first title element in tree order to change the title -- others have no effect.
     1572    if (m_titleElement) {
     1573        if (isHTMLDocument() || isXHTMLDocument())
     1574            m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first();
     1575        else if (isSVGDocument())
     1576            m_titleElement = descendantsOfType<SVGTitleElement>(*this).first();
     1577    } else
     1578        m_titleElement = newTitleElement;
     1579
     1580    updateTitleFromTitleElement();
     1581}
     1582
     1583void Document::titleElementAdded(Element& titleElement)
     1584{
     1585    if (m_titleElement == &titleElement)
     1586        return;
     1587
     1588    updateTitleElement(&titleElement);
     1589}
     1590
     1591void Document::titleElementRemoved(Element& titleElement)
     1592{
     1593    if (m_titleElement != &titleElement)
     1594        return;
     1595
     1596    updateTitleElement(nullptr);
     1597}
     1598
     1599void Document::titleElementTextChanged(Element& titleElement)
     1600{
     1601    if (m_titleElement != &titleElement)
     1602        return;
     1603
     1604    updateTitleFromTitleElement();
    15851605}
    15861606

  • trunk/Source/WebCore/dom/Document.h

    r189677 r189680  
    833833    void setTitle(const String&);
    834834
    835     void setTitleElement(const StringWithDirection&, Element* titleElement);
    836     void removeTitle(Element* titleElement);
     835    void titleElementAdded(Element& titleElement);
     836    void titleElementRemoved(Element& titleElement);
     837    void titleElementTextChanged(Element& titleElement);
    837838
    838839    String cookie(ExceptionCode&);
    … …  
    12881289    friend class IgnoreDestructiveWriteCountIncrementer;
    12891290
     1291    void updateTitleElement(Element* newTitleElement);
     1292
    12901293    void commonTeardown();
    12911294
    … …  
    13211324    virtual double timerAlignmentInterval(bool hasReachedMaxNestingLevel) const override final;
    13221325
     1326    void updateTitleFromTitleElement();
    13231327    void updateTitle(const StringWithDirection&);
    13241328    void updateFocusAppearanceTimerFired();
    … …  
    14771481    StringWithDirection m_title;
    14781482    StringWithDirection m_rawTitle;
    1479     bool m_titleSetExplicitly;
    14801483    RefPtr<Element> m_titleElement;
    14811484

  • trunk/Source/WebCore/html/HTMLTitleElement.cpp

    r179143 r189680  
    5454    HTMLElement::insertedInto(insertionPoint);
    5555    if (inDocument() && !isInShadowTree())
    56         document().setTitleElement(m_title, this);
     56        document().titleElementAdded(*this);
    5757    return InsertionDone;
    5858}
    … …  
    6262    HTMLElement::removedFrom(insertionPoint);
    6363    if (insertionPoint.inDocument() && !insertionPoint.isInShadowTree())
    64         document().removeTitle(this);
     64        document().titleElementRemoved(*this);
    6565}
    6666
    … …  
    6868{
    6969    HTMLElement::childrenChanged(change);
    70     m_title = textWithDirection();
    71     if (inDocument()) {
    72         if (!isInShadowTree())
    73             document().setTitleElement(m_title, this);
    74         else
    75             document().removeTitle(this);
    76     }
     70    m_title = computedTextWithDirection();
     71    document().titleElementTextChanged(*this);
    7772}
    7873
    … …  
    8277}
    8378
    84 StringWithDirection HTMLTitleElement::textWithDirection()
     79StringWithDirection HTMLTitleElement::computedTextWithDirection()
    8580{
    8681    TextDirection direction = LTR;

  • trunk/Source/WebCore/html/HTMLTitleElement.h

    r177996 r189680  
    3535    void setText(const String&);
    3636
    37     StringWithDirection textWithDirection();
     37    const StringWithDirection& textWithDirection() const { return m_title; }
    3838
    3939private:
    … …  
    4444    virtual void childrenChanged(const ChildChange&) override;
    4545
     46    StringWithDirection computedTextWithDirection();
     47
    4648    StringWithDirection m_title;
    4749};

  • trunk/Source/WebCore/svg/SVGTitleElement.cpp

    r178048 r189680  
    4444        return InsertionDone;
    4545
    46     if (firstChild() && document().isSVGDocument()) {
    47         // FIXME: does SVG have a title text direction?
    48         document().setTitleElement(StringWithDirection(textContent(), LTR), this);
    49     }
     46    if (firstChild() && document().isSVGDocument())
     47        document().titleElementAdded(*this);
    5048    return InsertionDone;
    5149}
    … …  
    5553    SVGElement::removedFrom(rootParent);
    5654    if (rootParent.inDocument() && document().isSVGDocument())
    57         document().removeTitle(this);
     55        document().titleElementRemoved(*this);
    5856}
    5957
    … …  
    6159{
    6260    SVGElement::childrenChanged(change);
    63     if (inDocument() && document().isSVGDocument()) {
    64         // FIXME: does SVG have title text direction?
    65         document().setTitleElement(StringWithDirection(textContent(), LTR), this);
    66     }
     61    document().titleElementTextChanged(*this);
    6762}
    6863
Note: See TracChangeset for help on using the changeset viewer.