- Revision
- 205689
- Author
- [email protected]
- Date
- 2016-09-08 20:31:21 -0700 (Thu, 08 Sep 2016)
Log 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:
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (205688 => 205689)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2016-09-09 03:31:21 UTC (rev 205689)
@@ -1,5 +1,17 @@
2016-09-08 Chris Dumez <[email protected]>
+ 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.
+
+ 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:
+
+2016-09-08 Chris Dumez <[email protected]>
+
Add support for HTMLIframeElement.allowFullscreen attribute
https://bugs.webkit.org/show_bug.cgi?id=161757
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt (205688 => 205689)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt 2016-09-09 03:31:21 UTC (rev 205689)
@@ -16,11 +16,11 @@
PASS Default start value for non-reversed list should be 1
PASS IDL and content attribute parse start of '.5' correctly.
PASS IDL and content attribute parse start of 'A' correctly.
-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
-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
-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
-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
-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
+PASS Default start value (if none provided) for reversed list = child li elements.
+PASS Default start value (if failed to parse) for reversed list = child li elements.
+PASS Default start value for reversed list = child li elements (even with tons of other child elements).
+PASS Adding child element to reversed list adds 1 to start value
+PASS Deleting child element from reversed list reduces start value by 1
PASS IDL and content attribute parse start of '2' correctly.
PASS IDL and content attribute parse start of '-10' correctly.
PASS IDL and content attribute parse start of '4.03' correctly.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt (205688 => 205689)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt 2016-09-09 03:31:21 UTC (rev 205689)
@@ -2,5 +2,5 @@
Two
One
-FAIL ol.start - reflection test assert_equals: expected 3 but got 0
+PASS ol.start - reflection test
Modified: trunk/Source/WebCore/ChangeLog (205688 => 205689)
--- trunk/Source/WebCore/ChangeLog 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/Source/WebCore/ChangeLog 2016-09-09 03:31:21 UTC (rev 205689)
@@ -1,5 +1,22 @@
2016-09-08 Chris Dumez <[email protected]>
+ 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.
+
+ 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:
+
+2016-09-08 Chris Dumez <[email protected]>
+
Add support for HTMLIframeElement.allowFullscreen attribute
https://bugs.webkit.org/show_bug.cgi?id=161757
Modified: trunk/Source/WebCore/html/HTMLOListElement.cpp (205688 => 205689)
--- trunk/Source/WebCore/html/HTMLOListElement.cpp 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/Source/WebCore/html/HTMLOListElement.cpp 2016-09-09 03:31:21 UTC (rev 205689)
@@ -94,7 +94,7 @@
HTMLElement::parseAttribute(name, value);
}
-void HTMLOListElement::setStart(int start)
+void HTMLOListElement::setStartForBindings(int start)
{
setIntegralAttribute(startAttr, start);
}
@@ -104,6 +104,16 @@
RenderListItem::updateItemValuesForOrderedList(*this);
}
+unsigned HTMLOListElement::itemCount(ShouldLayout shouldLayout) const
+{
+ if (shouldLayout == ShouldLayout::Yes)
+ document().updateLayoutIgnorePendingStylesheets();
+
+ if (m_shouldRecalculateItemCount)
+ const_cast<HTMLOListElement*>(this)->recalculateItemCount();
+ return m_itemCount;
+}
+
void HTMLOListElement::recalculateItemCount()
{
m_itemCount = RenderListItem::itemCountForOrderedList(*this);
Modified: trunk/Source/WebCore/html/HTMLOListElement.h (205688 => 205689)
--- trunk/Source/WebCore/html/HTMLOListElement.h 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/Source/WebCore/html/HTMLOListElement.h 2016-09-09 03:31:21 UTC (rev 205689)
@@ -32,9 +32,14 @@
static Ref<HTMLOListElement> create(Document&);
static Ref<HTMLOListElement> create(const QualifiedName&, Document&);
- int start() const { return m_start ? m_start.value() : (m_isReversed ? itemCount() : 1); }
- WEBCORE_EXPORT void setStart(int);
+ // FIXME: The reason we have this start() function which does not trigger layout is because it is called
+ // from rendering code and this is unfortunately one of the few cases where the render tree is mutated
+ // while in layout.
+ int start() const { return m_start ? m_start.value() : (m_isReversed ? itemCount(ShouldLayout::No) : 1); }
+ int startForBindings() const { return m_start ? m_start.value() : (m_isReversed ? itemCount(ShouldLayout::Yes) : 1); }
+ WEBCORE_EXPORT void setStartForBindings(int);
+
bool isReversed() const { return m_isReversed; }
void itemCountChanged() { m_shouldRecalculateItemCount = true; }
@@ -44,12 +49,8 @@
void updateItemValues();
- unsigned itemCount() const
- {
- if (m_shouldRecalculateItemCount)
- const_cast<HTMLOListElement*>(this)->recalculateItemCount();
- return m_itemCount;
- }
+ enum class ShouldLayout { No, Yes };
+ WEBCORE_EXPORT unsigned itemCount(ShouldLayout) const;
WEBCORE_EXPORT void recalculateItemCount();
Modified: trunk/Source/WebCore/html/HTMLOListElement.idl (205688 => 205689)
--- trunk/Source/WebCore/html/HTMLOListElement.idl 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/Source/WebCore/html/HTMLOListElement.idl 2016-09-09 03:31:21 UTC (rev 205689)
@@ -19,7 +19,7 @@
interface HTMLOListElement : HTMLElement {
[Reflect] attribute boolean compact;
- attribute long start;
+ [ImplementedAs=startForBindings] attribute long start;
[Reflect] attribute boolean reversed;
[Reflect] attribute DOMString type;
};
Modified: trunk/Source/WebKit/mac/DOM/DOMHTMLOListElement.mm (205688 => 205689)
--- trunk/Source/WebKit/mac/DOM/DOMHTMLOListElement.mm 2016-09-09 02:50:02 UTC (rev 205688)
+++ trunk/Source/WebKit/mac/DOM/DOMHTMLOListElement.mm 2016-09-09 03:31:21 UTC (rev 205689)
@@ -54,13 +54,13 @@
- (int)start
{
WebCore::JSMainThreadNullState state;
- return IMPL->start();
+ return IMPL->startForBindings();
}
- (void)setStart:(int)newStart
{
WebCore::JSMainThreadNullState state;
- IMPL->setStart(newStart);
+ IMPL->setStartForBindings(newStart);
}
- (BOOL)reversed
