Changeset 189804 in webkit


Ignore:
Timestamp:
Sep 15, 2015 1:57:56 AM (9 years ago)
Author:
Carlos Garcia Campos
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().

Location:
releases/WebKitGTK/webkit-2.10
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • releases/WebKitGTK/webkit-2.10/LayoutTests/imported/w3c/ChangeLog

    r189803 r189804  
     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

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog

    r189803 r189804  
     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

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.cpp

    r189803 r189804  
    131131#include "SVGElementFactory.h"
    132132#include "SVGNames.h"
     133#include "SVGTitleElement.h"
    133134#include "SchemeRegistry.h"
    134135#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)
    … …  
    15231523}
    15241524
     1525void Document::updateTitleFromTitleElement()
     1526{
     1527    if (!m_titleElement) {
     1528        updateTitle(StringWithDirection());
     1529        return;
     1530    }
     1531
     1532    if (is<HTMLTitleElement>(*m_titleElement))
     1533        updateTitle(downcast<HTMLTitleElement>(*m_titleElement).textWithDirection());
     1534    else if (is<SVGTitleElement>(*m_titleElement)) {
     1535        // FIXME: does SVG have a title text direction?
     1536        updateTitle(StringWithDirection(downcast<SVGTitleElement>(*m_titleElement).textContent(), LTR));
     1537    }
     1538}
     1539
    15251540void Document::setTitle(const String& title)
    15261541{
    1527     // Title set by JavaScript -- overrides any title elements.
    1528     m_titleSetExplicitly = true;
    15291542    if (!isHTMLDocument() && !isXHTMLDocument())
    15301543        m_titleElement = nullptr;
    … …  
    15441557}
    15451558
    1546 void Document::setTitleElement(const StringWithDirection& title, Element* titleElement)
    1547 {
    1548     if (titleElement != m_titleElement) {
    1549         if (m_titleElement || m_titleSetExplicitly) {
    1550             // Only allow the first title element to change the title -- others have no effect.
    1551             return;
    1552         }
    1553         m_titleElement = titleElement;
    1554     }
    1555 
    1556     updateTitle(title);
    1557 }
    1558 
    1559 void Document::removeTitle(Element* titleElement)
    1560 {
    1561     if (m_titleElement != titleElement)
    1562         return;
    1563 
    1564     m_titleElement = nullptr;
    1565     m_titleSetExplicitly = false;
    1566 
    1567     // Update title based on first title element in the head, if one exists.
    1568     if (HTMLElement* headElement = head()) {
    1569         if (auto firstTitle = childrenOfType<HTMLTitleElement>(*headElement).first())
    1570             setTitleElement(firstTitle->textWithDirection(), firstTitle);
    1571     }
    1572 
    1573     if (!m_titleElement)
    1574         updateTitle(StringWithDirection());
     1559void Document::updateTitleElement(Element* newTitleElement)
     1560{
     1561    // Only allow the first title element in tree order to change the title -- others have no effect.
     1562    if (m_titleElement) {
     1563        if (isHTMLDocument() || isXHTMLDocument())
     1564            m_titleElement = descendantsOfType<HTMLTitleElement>(*this).first();
     1565        else if (isSVGDocument())
     1566            m_titleElement = descendantsOfType<SVGTitleElement>(*this).first();
     1567    } else
     1568        m_titleElement = newTitleElement;
     1569
     1570    updateTitleFromTitleElement();
     1571}
     1572
     1573void Document::titleElementAdded(Element& titleElement)
     1574{
     1575    if (m_titleElement == &titleElement)
     1576        return;
     1577
     1578    updateTitleElement(&titleElement);
     1579}
     1580
     1581void Document::titleElementRemoved(Element& titleElement)
     1582{
     1583    if (m_titleElement != &titleElement)
     1584        return;
     1585
     1586    updateTitleElement(nullptr);
     1587}
     1588
     1589void Document::titleElementTextChanged(Element& titleElement)
     1590{
     1591    if (m_titleElement != &titleElement)
     1592        return;
     1593
     1594    updateTitleFromTitleElement();
    15751595}
    15761596

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/dom/Document.h

    r189802 r189804  
    837837    void setTitle(const String&);
    838838
    839     void setTitleElement(const StringWithDirection&, Element* titleElement);
    840     void removeTitle(Element* titleElement);
     839    void titleElementAdded(Element& titleElement);
     840    void titleElementRemoved(Element& titleElement);
     841    void titleElementTextChanged(Element& titleElement);
    841842
    842843    String cookie(ExceptionCode&);
    … …  
    12921293    friend class IgnoreDestructiveWriteCountIncrementer;
    12931294
     1295    void updateTitleElement(Element* newTitleElement);
     1296
    12941297    void commonTeardown();
    12951298
    … …  
    13251328    virtual double timerAlignmentInterval(bool hasReachedMaxNestingLevel) const override final;
    13261329
     1330    void updateTitleFromTitleElement();
    13271331    void updateTitle(const StringWithDirection&);
    13281332    void updateFocusAppearanceTimerFired();
    … …  
    14801484    StringWithDirection m_title;
    14811485    StringWithDirection m_rawTitle;
    1482     bool m_titleSetExplicitly;
    14831486    RefPtr<Element> m_titleElement;
    14841487

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.cpp

    r179143 r189804  
    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;

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/html/HTMLTitleElement.h

    r177996 r189804  
    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};

  • releases/WebKitGTK/webkit-2.10/Source/WebCore/svg/SVGTitleElement.cpp

    r178048 r189804  
    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.