Changeset 205689 in webkit


Ignore:
Timestamp:
Sep 8, 2016 8:31:21 PM (8 years ago)
Author:
Chris Dumez
Message:

ol.start may return incorrect value for reversed lists when not explicitly set
https://bugs.webkit.org/show_bug.cgi?id=161713

Reviewed by Zalan Bujtas.

LayoutTests/imported/w3c:

Rebaseline several W3C tests now that more checks are passing.

  • web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt:
  • web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt:

Source/WebCore:

ol.start may return incorrect value for reversed lists when not explicitly set.
This is because we're supposed to return the number of rendered <li> child
elements, which relies on layout. However, we did not make sure the layout is
up-to-date before counting the number of li child elements. This patch fixes
the issue.

No new tests, rebaselined existing tests.

  • html/HTMLOListElement.h:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/imported/w3c/ChangeLog

    r205686 r205689  
     12016-09-08  Chris Dumez  <cdumez@apple.com>
     2
     3        ol.start may return incorrect value for reversed lists when not explicitly set
     4        https://bugs.webkit.org/show_bug.cgi?id=161713
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Rebaseline several W3C tests now that more checks are passing.
     9
     10        * web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt:
     11        * web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt:
     12
    1132016-09-08  Chris Dumez  <cdumez@apple.com>
    214
  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt

    r204090 r205689  
    1717PASS IDL and content attribute parse start of '.5' correctly.
    1818PASS IDL and content attribute parse start of 'A' correctly.
    19 FAIL Default start value (if none provided) for reversed list = child li elements. assert_equals: no start attribute provided -> 3 expected 3 but got 0
    20 FAIL Default start value (if failed to parse) for reversed list = child li elements. assert_equals: start of A -> 3 (default) expected 3 but got 0
    21 FAIL Default start value for reversed list = child li elements (even with tons of other child elements). assert_equals: no start attribute -> 3 (default) expected 3 but got 0
    22 FAIL Adding child element to reversed list adds 1 to start value assert_equals: Adding child element to reversed list adds 1 to start value expected 4 but got 0
    23 FAIL Deleting child element from reversed list reduces start value by 1 assert_equals: Deleting child element from reversed list reduces start value by 1 expected 2 but got 0
     19PASS Default start value (if none provided) for reversed list = child li elements.
     20PASS Default start value (if failed to parse) for reversed list = child li elements.
     21PASS Default start value for reversed list = child li elements (even with tons of other child elements).
     22PASS Adding child element to reversed list adds 1 to start value
     23PASS Deleting child element from reversed list reduces start value by 1
    2424PASS IDL and content attribute parse start of '2' correctly.
    2525PASS IDL and content attribute parse start of '-10' correctly.
  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt

    r189476 r205689  
    33One
    44
    5 FAIL ol.start - reflection test assert_equals: expected 3 but got 0
     5PASS ol.start - reflection test
    66
  • trunk/Source/WebCore/ChangeLog

    r205686 r205689  
     12016-09-08  Chris Dumez  <cdumez@apple.com>
     2
     3        ol.start may return incorrect value for reversed lists when not explicitly set
     4        https://bugs.webkit.org/show_bug.cgi?id=161713
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        ol.start may return incorrect value for reversed lists when not explicitly set.
     9        This is because we're supposed to return the number of rendered <li> child
     10        elements, which relies on layout. However, we did not make sure the layout is
     11        up-to-date before counting the number of li child elements. This patch fixes
     12        the issue.
     13
     14        No new tests, rebaselined existing tests.
     15
     16        * html/HTMLOListElement.h:
     17
    1182016-09-08  Chris Dumez  <cdumez@apple.com>
    219
  • trunk/Source/WebCore/html/HTMLOListElement.cpp

    r197389 r205689  
    9595}
    9696
    97 void HTMLOListElement::setStart(int start)
     97void HTMLOListElement::setStartForBindings(int start)
    9898{
    9999    setIntegralAttribute(startAttr, start);
     
    105105}
    106106
     107unsigned HTMLOListElement::itemCount(ShouldLayout shouldLayout) const
     108{
     109    if (shouldLayout == ShouldLayout::Yes)
     110        document().updateLayoutIgnorePendingStylesheets();
     111
     112    if (m_shouldRecalculateItemCount)
     113        const_cast<HTMLOListElement*>(this)->recalculateItemCount();
     114    return m_itemCount;
     115}
     116
    107117void HTMLOListElement::recalculateItemCount()
    108118{
  • trunk/Source/WebCore/html/HTMLOListElement.h

    r204717 r205689  
    3333    static Ref<HTMLOListElement> create(const QualifiedName&, Document&);
    3434
    35     int start() const { return m_start ? m_start.value() : (m_isReversed ? itemCount() : 1); }
    36     WEBCORE_EXPORT void setStart(int);
     35    // FIXME: The reason we have this start() function which does not trigger layout is because it is called
     36    // from rendering code and this is unfortunately one of the few cases where the render tree is mutated
     37    // while in layout.
     38    int start() const { return m_start ? m_start.value() : (m_isReversed ? itemCount(ShouldLayout::No) : 1); }
     39    int startForBindings() const { return m_start ? m_start.value() : (m_isReversed ? itemCount(ShouldLayout::Yes) : 1); }
     40
     41    WEBCORE_EXPORT void setStartForBindings(int);
    3742
    3843    bool isReversed() const { return m_isReversed; }
     
    4550    void updateItemValues();
    4651
    47     unsigned itemCount() const
    48     {
    49         if (m_shouldRecalculateItemCount)
    50             const_cast<HTMLOListElement*>(this)->recalculateItemCount();
    51         return m_itemCount;
    52     }
     52    enum class ShouldLayout { No, Yes };
     53    WEBCORE_EXPORT unsigned itemCount(ShouldLayout) const;
    5354
    5455    WEBCORE_EXPORT void recalculateItemCount();
  • trunk/Source/WebCore/html/HTMLOListElement.idl

    r131172 r205689  
    2020interface HTMLOListElement : HTMLElement {
    2121    [Reflect] attribute boolean compact;
    22     attribute long start;
     22    [ImplementedAs=startForBindings] attribute long start;
    2323    [Reflect] attribute boolean reversed;
    2424    [Reflect] attribute DOMString type;
  • trunk/Source/WebKit/mac/DOM/DOMHTMLOListElement.mm

    r204717 r205689  
    5555{
    5656    WebCore::JSMainThreadNullState state;
    57     return IMPL->start();
     57    return IMPL->startForBindings();
    5858}
    5959
     
    6161{
    6262    WebCore::JSMainThreadNullState state;
    63     IMPL->setStart(newStart);
     63    IMPL->setStartForBindings(newStart);
    6464}
    6565
Note: See TracChangeset for help on using the changeset viewer.