- Revision
- 223036
- Author
- [email protected]
- Date
- 2017-10-08 17:11:50 -0700 (Sun, 08 Oct 2017)
Log Message
Update HTMLOListElement.start to behavior from latest HTML specification
https://bugs.webkit.org/show_bug.cgi?id=178057
Reviewed by Chris Dumez.
LayoutTests/imported/w3c:
* 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:
Updated to expect more tests to pass.
Source/WebCore:
* html/HTMLOListElement.cpp:
(optionalValue): Added. Helper function that we can put into Expected.h later
if we like; makes it easier to turn Expected into std::optional.
(WebCore::HTMLOListElement::HTMLOListElement): Moved data member initialization
into class definition so it doesn't have to be done here.
(WebCore::HTMLOListElement::parseAttribute): Simplified using the new
optionalValue function. Moved the call to update values in here since it's
a trivial one-liner (albeit done twice).
(WebCore::HTMLOListElement::updateItemValues): Deleted. Moved this into the
parseAttribute function.
(WebCore::HTMLOListElement::itemCount): Updated to use std::optional instead
of a separate m_shouldRecalculateItemCount flag. Also inlined the
recalculateItemCount function since it's a trivial one-liner.
(WebCore::HTMLOListElement::itemCountAfterLayout): Deleted. The only use of
this was to implement the now-obsolete behavior of the start attribute.
(WebCore::HTMLOListElement::recalculateItemCount): Deleted. Moved this into
the itemCount function.
* html/HTMLOListElement.h: Changed startForBindings to return 1 when start
is not specified; this what the HTML specification now calls for. Updated
for the changes above. Merged m_itemCount and m_shouldRecalculateItemCount
into a single optional m_itemCount, and made it mutable so it can be
computed as a side effect of calling the const member function start.
LayoutTests:
* fast/lists/ol-reversed-simple-expected.txt:
* fast/lists/ol-reversed-simple.html:
* fast/lists/ol-reversed-simple.xhtml:
Updated test and results to expect the new behavior.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (223035 => 223036)
--- trunk/LayoutTests/ChangeLog 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/ChangeLog 2017-10-09 00:11:50 UTC (rev 223036)
@@ -1,5 +1,17 @@
2017-10-08 Darin Adler <[email protected]>
+ Update HTMLOListElement.start to behavior from latest HTML specification
+ https://bugs.webkit.org/show_bug.cgi?id=178057
+
+ Reviewed by Chris Dumez.
+
+ * fast/lists/ol-reversed-simple-expected.txt:
+ * fast/lists/ol-reversed-simple.html:
+ * fast/lists/ol-reversed-simple.xhtml:
+ Updated test and results to expect the new behavior.
+
+2017-10-08 Darin Adler <[email protected]>
+
Fix bugs related to setting reflected floating point DOM attributes
https://bugs.webkit.org/show_bug.cgi?id=178061
Modified: trunk/LayoutTests/fast/lists/ol-reversed-simple-expected.txt (223035 => 223036)
--- trunk/LayoutTests/fast/lists/ol-reversed-simple-expected.txt 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/fast/lists/ol-reversed-simple-expected.txt 2017-10-09 00:11:50 UTC (rev 223036)
@@ -41,6 +41,6 @@
-8 Minus Eight
-9 Minus Nine
-This tests that reversed lists have the start attribute equals the number of list items when no start value is specified.
+This tests that reversed lists have a start attribute of 1 when no start value is specified.
-Value of start attribute of the list is : 5
+Value of start attribute of the list is : 1
Modified: trunk/LayoutTests/fast/lists/ol-reversed-simple.html (223035 => 223036)
--- trunk/LayoutTests/fast/lists/ol-reversed-simple.html 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/fast/lists/ol-reversed-simple.html 2017-10-09 00:11:50 UTC (rev 223036)
@@ -89,7 +89,7 @@
</ol>
<p id="console5"></p>
- <p>This tests that reversed lists have the start attribute equals the number of list items when no start value is specified.</p>
+ <p>This tests that reversed lists have a start attribute of 1 when no start value is specified.</p>
<ol id="list6" reversed>
<li>Five</li>
<li>Four</li>
Modified: trunk/LayoutTests/fast/lists/ol-reversed-simple.xhtml (223035 => 223036)
--- trunk/LayoutTests/fast/lists/ol-reversed-simple.xhtml 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/fast/lists/ol-reversed-simple.xhtml 2017-10-09 00:11:50 UTC (rev 223036)
@@ -90,7 +90,7 @@
</ol>
<p id="console5"></p>
- <p>This tests that reversed lists have the start attribute equals the number of list items when no start value is specified.</p>
+ <p>This tests that reversed lists have a start attribute of 1 when no start value is specified.</p>
<ol id="list6" reversed="reversed">
<li>Five</li>
<li>Four</li>
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (223035 => 223036)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2017-10-09 00:11:50 UTC (rev 223036)
@@ -1,3 +1,14 @@
+2017-10-07 Darin Adler <[email protected]>
+
+ Update HTMLOListElement.start to behavior from latest HTML specification
+ https://bugs.webkit.org/show_bug.cgi?id=178057
+
+ Reviewed by Chris Dumez.
+
+ * 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:
+ Updated to expect more tests to pass.
+
2017-10-08 Darin Adler <[email protected]>
Fix bugs related to setting reflected floating point DOM attributes
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt (223035 => 223036)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/grouping-ol-expected.txt 2017-10-09 00:11:50 UTC (rev 223036)
@@ -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 = 1. assert_equals: expected 1 but got 3
-FAIL Default start value (if failed to parse) for reversed list = 1. assert_equals: expected 1 but got 3
-FAIL Default start value for reversed list = 1 (even with tons of other child elements). assert_equals: expected 1 but got 3
-FAIL Adding child element to reversed list does not change start value assert_equals: expected 1 but got 4
-FAIL Deleting child element from reversed list does not change start value assert_equals: expected 1 but got 2
+PASS Default start value (if none provided) for reversed list = 1.
+PASS Default start value (if failed to parse) for reversed list = 1.
+PASS Default start value for reversed list = 1 (even with tons of other child elements).
+PASS Adding child element to reversed list does not change start value
+PASS Deleting child element from reversed list does not change start value
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 (223035 => 223036)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/grouping-content/the-ol-element/ol.start-reflection-2-expected.txt 2017-10-09 00:11:50 UTC (rev 223036)
@@ -2,5 +2,5 @@
Two
One
-FAIL ol.start - reflection test assert_equals: expected 1 but got 3
+PASS ol.start - reflection test
Modified: trunk/Source/WebCore/ChangeLog (223035 => 223036)
--- trunk/Source/WebCore/ChangeLog 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/Source/WebCore/ChangeLog 2017-10-09 00:11:50 UTC (rev 223036)
@@ -1,5 +1,36 @@
2017-10-08 Darin Adler <[email protected]>
+ Update HTMLOListElement.start to behavior from latest HTML specification
+ https://bugs.webkit.org/show_bug.cgi?id=178057
+
+ Reviewed by Chris Dumez.
+
+ * html/HTMLOListElement.cpp:
+ (optionalValue): Added. Helper function that we can put into Expected.h later
+ if we like; makes it easier to turn Expected into std::optional.
+ (WebCore::HTMLOListElement::HTMLOListElement): Moved data member initialization
+ into class definition so it doesn't have to be done here.
+ (WebCore::HTMLOListElement::parseAttribute): Simplified using the new
+ optionalValue function. Moved the call to update values in here since it's
+ a trivial one-liner (albeit done twice).
+ (WebCore::HTMLOListElement::updateItemValues): Deleted. Moved this into the
+ parseAttribute function.
+ (WebCore::HTMLOListElement::itemCount): Updated to use std::optional instead
+ of a separate m_shouldRecalculateItemCount flag. Also inlined the
+ recalculateItemCount function since it's a trivial one-liner.
+ (WebCore::HTMLOListElement::itemCountAfterLayout): Deleted. The only use of
+ this was to implement the now-obsolete behavior of the start attribute.
+ (WebCore::HTMLOListElement::recalculateItemCount): Deleted. Moved this into
+ the itemCount function.
+
+ * html/HTMLOListElement.h: Changed startForBindings to return 1 when start
+ is not specified; this what the HTML specification now calls for. Updated
+ for the changes above. Merged m_itemCount and m_shouldRecalculateItemCount
+ into a single optional m_itemCount, and made it mutable so it can be
+ computed as a side effect of calling the const member function start.
+
+2017-10-08 Darin Adler <[email protected]>
+
Fix bugs related to setting reflected floating point DOM attributes
https://bugs.webkit.org/show_bug.cgi?id=178061
Modified: trunk/Source/WebCore/html/HTMLOListElement.cpp (223035 => 223036)
--- trunk/Source/WebCore/html/HTMLOListElement.cpp 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/Source/WebCore/html/HTMLOListElement.cpp 2017-10-09 00:11:50 UTC (rev 223036)
@@ -29,15 +29,19 @@
#include "HTMLParserIdioms.h"
#include "RenderListItem.h"
+// FIXME: There should be a standard way to turn a std::expected into a std::optional.
+// Maybe we should put this into the header file for Expected and give it a better name.
+template<typename T, typename E> inline std::optional<T> optionalValue(Expected<T, E>&& expected)
+{
+ return expected ? std::optional<T>(WTFMove(expected.value())) : std::nullopt;
+}
+
namespace WebCore {
using namespace HTMLNames;
-HTMLOListElement::HTMLOListElement(const QualifiedName& tagName, Document& document)
+inline HTMLOListElement::HTMLOListElement(const QualifiedName& tagName, Document& document)
: HTMLElement(tagName, document)
- , m_itemCount(0)
- , m_isReversed(false)
- , m_shouldRecalculateItemCount(false)
{
ASSERT(hasTagName(olTag));
}
@@ -80,17 +84,16 @@
{
if (name == startAttr) {
int oldStart = start();
- auto optionalStart = parseHTMLInteger(value);
- m_start = optionalStart ? std::optional<int>(optionalStart.value()) : std::nullopt;
+ m_start = optionalValue(parseHTMLInteger(value));
if (oldStart == start())
return;
- updateItemValues();
+ RenderListItem::updateItemValuesForOrderedList(*this);
} else if (name == reversedAttr) {
bool reversed = !value.isNull();
if (reversed == m_isReversed)
return;
m_isReversed = reversed;
- updateItemValues();
+ RenderListItem::updateItemValuesForOrderedList(*this);
} else
HTMLElement::parseAttribute(name, value);
}
@@ -100,29 +103,11 @@
setIntegralAttribute(startAttr, start);
}
-void HTMLOListElement::updateItemValues()
-{
- RenderListItem::updateItemValuesForOrderedList(*this);
-}
-
unsigned HTMLOListElement::itemCount() const
{
- if (m_shouldRecalculateItemCount)
- const_cast<HTMLOListElement*>(this)->recalculateItemCount();
- return m_itemCount;
+ if (!m_itemCount)
+ m_itemCount = RenderListItem::itemCountForOrderedList(*this);
+ return m_itemCount.value();
}
-unsigned HTMLOListElement::itemCountAfterLayout() const
-{
- document().updateLayoutIgnorePendingStylesheets();
-
- return itemCount();
}
-
-void HTMLOListElement::recalculateItemCount()
-{
- m_itemCount = RenderListItem::itemCountForOrderedList(*this);
- m_shouldRecalculateItemCount = false;
-}
-
-}
Modified: trunk/Source/WebCore/html/HTMLOListElement.h (223035 => 223036)
--- trunk/Source/WebCore/html/HTMLOListElement.h 2017-10-08 22:17:46 UTC (rev 223035)
+++ trunk/Source/WebCore/html/HTMLOListElement.h 2017-10-09 00:11:50 UTC (rev 223036)
@@ -1,7 +1,7 @@
/*
* Copyright (C) 1999 Lars Knoll ([email protected])
* (C) 1999 Antti Koivisto ([email protected])
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -23,7 +23,6 @@
#pragma once
#include "HTMLElement.h"
-#include <wtf/Optional.h>
namespace WebCore {
@@ -32,37 +31,29 @@
static Ref<HTMLOListElement> create(Document&);
static Ref<HTMLOListElement> create(const QualifiedName&, Document&);
- // 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() : 1); }
- int startForBindings() const { return m_start ? m_start.value() : (m_isReversed ? itemCountAfterLayout() : 1); }
-
+ int startForBindings() const { return m_start.value_or(1); }
WEBCORE_EXPORT void setStartForBindings(int);
+ // FIXME: The reason start() does not trigger layout is because it is called
+ // from rendering code. This is unfortunately one of the few cases where the
+ // render tree is mutated during layout.
+
+ int start() const { return m_start ? m_start.value() : (m_isReversed ? itemCount() : 1); }
bool isReversed() const { return m_isReversed; }
+ void itemCountChanged() { m_itemCount = std::nullopt; }
- void itemCountChanged() { m_shouldRecalculateItemCount = true; }
-
private:
HTMLOListElement(const QualifiedName&, Document&);
- void updateItemValues();
-
- WEBCORE_EXPORT unsigned itemCountAfterLayout() const;
WEBCORE_EXPORT unsigned itemCount() const;
- WEBCORE_EXPORT void recalculateItemCount();
-
void parseAttribute(const QualifiedName&, const AtomicString&) final;
bool isPresentationAttribute(const QualifiedName&) const final;
void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) final;
std::optional<int> m_start;
- unsigned m_itemCount;
-
- bool m_isReversed : 1;
- bool m_shouldRecalculateItemCount : 1;
+ mutable std::optional<unsigned> m_itemCount;
+ bool m_isReversed { false };
};
} //namespace
