WebKit Bugzilla
Attachment 340866 Details for
Bug 179391
: Make CSSOM APIs append rather than replace slots in the declaration block.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-179391-20180521205119.patch (text/plain), 21.43 KB, created by
Emilio Cobos Álvarez (:emilio)
on 2018-05-21 11:51:21 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Emilio Cobos Álvarez (:emilio)
Created:
2018-05-21 11:51:21 PDT
Size:
21.43 KB
patch
obsolete
>Subversion Revision: 232014 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index ca4f7c9cfc33bf72693533396321bdba9fd137af..d46db54b74bae37885c08954ccb75993e17f9713 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,8 +1,38 @@ >+2018-05-20 Emilio Cobos Ãlvarez <emilio@crisal.io> >+ >+ [cssom] Make CSSOM APIs always append to the declaration block. >+ https://bugs.webkit.org/show_bug.cgi?id=179391 >+ >+ This aligns us with Firefox and Blink which have already implemented >+ the change in [1] and [2] respectively. >+ >+ See [3] and [4] for spec text and discussion, and [5] for the blik-dev >+ intent to implement and ship thread for more context. >+ >+ I found a couple bits that don't really make sense from >+ addParsedProperty anymore, which will fix separately with separate >+ tests. >+ >+ [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1415330 >+ [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=782407 >+ [3]: https://drafts.csswg.org/cssom/#set-a-css-declaration, >+ [4]: https://github.com/w3c/csswg-drafts/issues/1898 >+ [5]: https://groups.google.com/a/chromium.org/d/msg/blink-dev/lzBoa1EAQpI/strfpNczAwAJ >+ >+ Tests: LayoutTests/imported/web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001.html >+ LayoutTests/imported/web-platform-tests/css/cssom/cssstyledeclaration-setter-order.html >+ >+ * css/StyleProperties.cpp: >+ (WebCore::MutableStyleProperties::setProperty): >+ (WebCore::MutableStyleProperties::setCustomProperty): >+ (WebCore::MutableStyleProperties::addParsedProperty): >+ * css/StyleProperties.h: >+ > 2018-05-21 Alicia Boya GarcÃa <aboya@igalia.com> > > [MSE][GStreamer] Force segment.start = 0 after matroskademux > https://bugs.webkit.org/show_bug.cgi?id=185740 > > Reviewed by Xabier Rodriguez-Calvar. > > This patch ensures that when WebM MSE media segments are appended in >diff --git a/Source/WebCore/css/StyleProperties.cpp b/Source/WebCore/css/StyleProperties.cpp >index 30dbeb42b6b5a67a06fa9c65e0fdfc6deccf9086..74b91c41cf6af64b96b3f75bbf381eac1c4397e1 100644 >--- a/Source/WebCore/css/StyleProperties.cpp >+++ b/Source/WebCore/css/StyleProperties.cpp >@@ -758,17 +758,16 @@ bool MutableStyleProperties::setProperty(CSSPropertyID propertyID, const String& > // Setting the value to an empty string just removes the property in both IE and Gecko. > // Setting it to null seems to produce less consistent results, but we treat it just the same. > if (value.isEmpty()) > return removeProperty(propertyID); > > parserContext.mode = cssParserMode(); > > // When replacing an existing property value, this moves the property to the end of the list. >- // Firefox preserves the position, and MSIE moves the property to the beginning. > return CSSParser::parseValue(*this, propertyID, value, important, parserContext) == CSSParser::ParseResult::Changed; > } > > bool MutableStyleProperties::setProperty(CSSPropertyID propertyID, const String& value, bool important) > { > CSSParserContext parserContext(cssParserMode()); > return setProperty(propertyID, value, important, parserContext); > } >@@ -777,17 +776,16 @@ bool MutableStyleProperties::setCustomProperty(const String& propertyName, const > { > // Setting the value to an empty string just removes the property in both IE and Gecko. > // Setting it to null seems to produce less consistent results, but we treat it just the same. > if (value.isEmpty()) > return removeCustomProperty(propertyName); > > parserContext.mode = cssParserMode(); > // When replacing an existing property value, this moves the property to the end of the list. >- // Firefox preserves the position, and MSIE moves the property to the beginning. > return CSSParser::parseCustomPropertyValue(*this, propertyName, value, important, parserContext) == CSSParser::ParseResult::Changed; > } > > void MutableStyleProperties::setProperty(CSSPropertyID propertyID, RefPtr<CSSValue>&& value, bool important) > { > StylePropertyShorthand shorthand = shorthandForProperty(propertyID); > if (!shorthand.length()) { > setProperty(CSSProperty(propertyID, WTFMove(value), important)); >@@ -795,49 +793,33 @@ void MutableStyleProperties::setProperty(CSSPropertyID propertyID, RefPtr<CSSVal > } > > removePropertiesInSet(shorthand.properties(), shorthand.length()); > > for (unsigned i = 0; i < shorthand.length(); ++i) > m_propertyVector.append(CSSProperty(shorthand.properties()[i], value.copyRef(), important)); > } > >-bool MutableStyleProperties::setProperty(const CSSProperty& property, CSSProperty* slot) >+void MutableStyleProperties::setProperty(const CSSProperty& property) > { >- if (!removeShorthandProperty(property.id())) { >- CSSProperty* toReplace = slot; >- if (!slot) { >- if (property.id() == CSSPropertyCustom) { >- if (property.value()) >- toReplace = findCustomCSSPropertyWithName(downcast<CSSCustomPropertyValue>(*property.value()).name()); >- } else >- toReplace = findCSSPropertyWithID(property.id()); >- } >- >- if (toReplace) { >- if (*toReplace == property) >- return false; >- >- *toReplace = property; >- return true; >- } >- } >- >+ if (property.id() == CSSPropertyCustom) >+ removeCustomProperty(downcast<CSSCustomPropertyValue>(*property.value()).name()); >+ else >+ removeProperty(property.id()); > m_propertyVector.append(property); >- return true; > } > >-bool MutableStyleProperties::setProperty(CSSPropertyID propertyID, CSSValueID identifier, bool important) >+void MutableStyleProperties::setProperty(CSSPropertyID propertyID, CSSValueID identifier, bool important) > { >- return setProperty(CSSProperty(propertyID, CSSValuePool::singleton().createIdentifierValue(identifier), important)); >+ setProperty(CSSProperty(propertyID, CSSValuePool::singleton().createIdentifierValue(identifier), important)); > } > >-bool MutableStyleProperties::setProperty(CSSPropertyID propertyID, CSSPropertyID identifier, bool important) >+void MutableStyleProperties::setProperty(CSSPropertyID propertyID, CSSPropertyID identifier, bool important) > { >- return setProperty(CSSProperty(propertyID, CSSValuePool::singleton().createIdentifierValue(identifier), important)); >+ setProperty(CSSProperty(propertyID, CSSValuePool::singleton().createIdentifierValue(identifier), important)); > } > > bool MutableStyleProperties::parseDeclaration(const String& styleDeclaration, CSSParserContext context) > { > auto oldProperties = WTFMove(m_propertyVector); > m_propertyVector.clear(); > > context.mode = cssParserMode(); >@@ -858,22 +840,29 @@ bool MutableStyleProperties::addParsedProperties(const ParsedPropertyVector& pro > anyChanged = true; > } > > return anyChanged; > } > > bool MutableStyleProperties::addParsedProperty(const CSSProperty& property) > { >+ // FIXME: This stopped looking at !important for non-custom properties in >+ // https://webkit.org/b/159959, which is what some callers, mainly >+ // mergeAndOverrideOnConflict, assume this function does. > if (property.id() == CSSPropertyCustom) { >- if ((property.value() && !customPropertyIsImportant(downcast<CSSCustomPropertyValue>(*property.value()).name())) || property.isImportant()) >- return setProperty(property); >- return false; >+ if (!property.value()) >+ return false; >+ if (!property.isImportant() && customPropertyIsImportant(downcast<CSSCustomPropertyValue>(*property.value()).name())) >+ return false; >+ setProperty(property); >+ return true; > } >- return setProperty(property); >+ setProperty(property); >+ return true; > } > > String StyleProperties::asText() const > { > StringBuilder result; > > int positionXPropertyIndex = -1; > int positionYPropertyIndex = -1; >diff --git a/Source/WebCore/css/StyleProperties.h b/Source/WebCore/css/StyleProperties.h >index 2661ec1121108a635065dc6a250145d6d5649a39..b6ade6e90a3c56ea3f59fc59c0924bf4c0e1f63b 100644 >--- a/Source/WebCore/css/StyleProperties.h >+++ b/Source/WebCore/css/StyleProperties.h >@@ -223,19 +223,19 @@ public: > bool addParsedProperty(const CSSProperty&); > > // These expand shorthand properties into multiple properties. > bool setProperty(CSSPropertyID, const String& value, bool important, CSSParserContext); > bool setProperty(CSSPropertyID, const String& value, bool important = false); > void setProperty(CSSPropertyID, RefPtr<CSSValue>&&, bool important = false); > > // These do not. FIXME: This is too messy, we can do better. >- bool setProperty(CSSPropertyID, CSSValueID identifier, bool important = false); >- bool setProperty(CSSPropertyID, CSSPropertyID identifier, bool important = false); >- bool setProperty(const CSSProperty&, CSSProperty* slot = nullptr); >+ void setProperty(CSSPropertyID, CSSValueID identifier, bool important = false); >+ void setProperty(CSSPropertyID, CSSPropertyID identifier, bool important = false); >+ void setProperty(const CSSProperty&); > > bool removeProperty(CSSPropertyID, String* returnText = nullptr); > void removeBlockProperties(); > bool removePropertiesInSet(const CSSPropertyID* set, unsigned length); > > void mergeAndOverrideOnConflict(const StyleProperties&); > > void clear(); >diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog >index d26a4a08b6064aca6719db3584802ca154a0d224..32ad9512b994ca32fb2c1500db1d4f577c0e8c09 100644 >--- a/LayoutTests/imported/w3c/ChangeLog >+++ b/LayoutTests/imported/w3c/ChangeLog >@@ -1,8 +1,20 @@ >+2018-05-20 Emilio Cobos Ãlvarez <emilio@crisal.io> >+ >+ [cssom] Make CSSOM APIs always append to the declaration block. >+ https://bugs.webkit.org/show_bug.cgi?id=179391 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Rebaselined tests to account for the progression. >+ >+ * web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001-expected.txt: >+ * web-platform-tests/css/cssom/cssstyledeclaration-setter-order-expected.txt: >+ > 2018-05-20 Emilio Cobos Ãlvarez <emilio@crisal.io> > > Update CSSOM WPT tests. > https://bugs.webkit.org/show_bug.cgi?id=185805 > > They've been moved under the css/ directory. > > This has been done with: >diff --git a/LayoutTests/editing/pasteboard/copy-paste-converts-fixed-expected.txt b/LayoutTests/editing/pasteboard/copy-paste-converts-fixed-expected.txt >index c169e94c9cafc47af2da611fbd92f805686e9542..fbd6aa62a20b62068d8f5abcdb937b33021a1df7 100644 >--- a/LayoutTests/editing/pasteboard/copy-paste-converts-fixed-expected.txt >+++ b/LayoutTests/editing/pasteboard/copy-paste-converts-fixed-expected.txt >@@ -66,17 +66,17 @@ first test - after: > " > | " > " > | <body> > | <div> > | style="position: relative;" > | "abc" > | <div> >-| style="position: absolute; left: 0px; top: 0px;" >+| style="left: 0px; top: 0px; position: absolute;" > | "def" > | "ghi <#selection-caret>" > | " > > " > | <script> > | " > >diff --git a/LayoutTests/editing/pasteboard/copy-paste-converts-sticky-expected.txt b/LayoutTests/editing/pasteboard/copy-paste-converts-sticky-expected.txt >index 9e98c6444d2a294b7ea29b439554ad5b07df2cba..9e9e4c02c961c5e4ce33d8abead66f2efa5ae64c 100644 >--- a/LayoutTests/editing/pasteboard/copy-paste-converts-sticky-expected.txt >+++ b/LayoutTests/editing/pasteboard/copy-paste-converts-sticky-expected.txt >@@ -64,17 +64,17 @@ first test - after: > | type="text/javascript" > | " > " > | " > " > | <body> > | "abc" > | <div> >-| style="position: static; left: 0px; top: 0px;" >+| style="left: 0px; top: 0px; position: static;" > | "def" > | "ghi <#selection-caret> > > " > | <script> > | " > > Markup.description('This tests to see if position:sticky gets converted to position:static upon copy/paste'); >diff --git a/LayoutTests/fast/css/set-inline-style-recalc-expected.txt b/LayoutTests/fast/css/set-inline-style-recalc-expected.txt >index 650db33b9d0eb999bdf6f27ff9f83d3cdfbd2ee8..651d8010001f9d0b2e44a586cef31c61bf371390 100644 >--- a/LayoutTests/fast/css/set-inline-style-recalc-expected.txt >+++ b/LayoutTests/fast/css/set-inline-style-recalc-expected.txt >@@ -1,21 +1,21 @@ > Test to count the number of recalcStyle calls on inline style and class changes. > > On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". > > > PASS numStyleRecalcs is 1 > PASS numStyleRecalcs is 1 >-PASS numStyleRecalcs is 0 > PASS numStyleRecalcs is 1 >-PASS numStyleRecalcs is 0 > PASS numStyleRecalcs is 1 >-PASS numStyleRecalcs is 0 > PASS numStyleRecalcs is 1 >-PASS numStyleRecalcs is 0 >+PASS numStyleRecalcs is 1 >+PASS numStyleRecalcs is 1 >+PASS numStyleRecalcs is 1 >+PASS numStyleRecalcs is 1 > PASS numStyleRecalcs is 1 > PASS numStyleRecalcs is 0 > PASS numStyleRecalcs is 0 > PASS successfullyParsed is true > > TEST COMPLETE > >diff --git a/LayoutTests/fast/css/set-inline-style-recalc.html b/LayoutTests/fast/css/set-inline-style-recalc.html >index 84103eb32a301504e87f493d24cab7d6e0821abf..55c7a9350c320e9170fa3918d9722e25fa449181 100644 >--- a/LayoutTests/fast/css/set-inline-style-recalc.html >+++ b/LayoutTests/fast/css/set-inline-style-recalc.html >@@ -42,32 +42,32 @@ document.body.appendChild(testContainer); > > var style = testContainer.style; > > var numStyleRecalcs = setStyleAndForceLayout(function() { style.height = "10px"; }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { style.height = "20px"; }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { style.height = "20px"; }); >-shouldBe("numStyleRecalcs", "0"); >+shouldBe("numStyleRecalcs", "1"); > > numStyleRecalcs = setStyleAndForceLayout(function() { style.backgroundPosition = "10px 10%"; }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { style.backgroundPosition = "10px 10%"; }); >-shouldBe("numStyleRecalcs", "0"); >+shouldBe("numStyleRecalcs", "1"); > > numStyleRecalcs = setStyleAndForceLayout(function() { style.backgroundColor = "red"; }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { style.backgroundColor = "red"; }); >-shouldBe("numStyleRecalcs", "0"); >+shouldBe("numStyleRecalcs", "1"); > > numStyleRecalcs = setStyleAndForceLayout(function() { style.transform = "translate(10px, 10px) rotate(10deg)"; }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { style.transform = "translate(10px, 10px) rotate(10deg)"; }); >-shouldBe("numStyleRecalcs", "0"); >+shouldBe("numStyleRecalcs", "1"); > > numStyleRecalcs = setStyleAndForceLayout(function() { testContainer.classList.add('foo'); }); > shouldBe("numStyleRecalcs", "1"); > numStyleRecalcs = setStyleAndForceLayout(function() { testContainer.classList.add('foo'); }); > shouldBe("numStyleRecalcs", "0"); > numStyleRecalcs = setStyleAndForceLayout(function() { testContainer.classList.add('bar'); }); > shouldBe("numStyleRecalcs", "0"); > >diff --git a/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt b/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt >index cb949aa33add2c0d668a12b201b539b56bcc7dbf..4fa92ac1c043190ee7b85e9f0064f4085cbd0325 100644 >--- a/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt >+++ b/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt >@@ -168,17 +168,17 @@ PASS mutations is null > > Testing that modifying an elements style property dispatches Mutation Records with correct oldValues. > PASS mutations.length is 3 > PASS mutations[0].type is "attributes" > PASS mutations[0].attributeName is "style" > PASS mutations[0].oldValue is "color: yellow; width: 100px;" > PASS mutations[1].type is "attributes" > PASS mutations[1].attributeName is "style" >-PASS mutations[1].oldValue is "color: red; width: 100px;" >+PASS mutations[1].oldValue is "width: 100px; color: red;" > PASS mutations[2].type is "attributes" > PASS mutations[2].attributeName is "style" > PASS mutations[2].oldValue is "color: red; width: 200px;" > ...mutation record created. > PASS mutations is null > > Testing that a no-op style property mutation does not create Mutation Records. > PASS mutations is null >diff --git a/LayoutTests/fast/dom/MutationObserver/observe-attributes.html b/LayoutTests/fast/dom/MutationObserver/observe-attributes.html >index cc5606ce82f28603b6d4dc7d61de84de26fce20e..b951326f5532b5416aae722e6cccb0277d098d25 100644 >--- a/LayoutTests/fast/dom/MutationObserver/observe-attributes.html >+++ b/LayoutTests/fast/dom/MutationObserver/observe-attributes.html >@@ -710,17 +710,17 @@ function testStyleAttributePropertyAccessOldValue() { > > function checkAndContinue() { > shouldBe('mutations.length', '3'); > shouldBe('mutations[0].type', '"attributes"'); > shouldBe('mutations[0].attributeName', '"style"'); > shouldBe('mutations[0].oldValue', '"color: yellow; width: 100px;"'); > shouldBe('mutations[1].type', '"attributes"'); > shouldBe('mutations[1].attributeName', '"style"'); >- shouldBe('mutations[1].oldValue', '"color: red; width: 100px;"'); >+ shouldBe('mutations[1].oldValue', '"width: 100px; color: red;"'); > shouldBe('mutations[2].type', '"attributes"'); > shouldBe('mutations[2].attributeName', '"style"'); > shouldBe('mutations[2].oldValue', '"color: red; width: 200px;"'); > > mutations = null; > div.getAttribute('style'); > setTimeout(finish, 0); > } >diff --git a/LayoutTests/fast/dom/css-set-property-exception-expected.txt b/LayoutTests/fast/dom/css-set-property-exception-expected.txt >index e7eb7ef39a9bbe670ff6bd183a795d9a8922e372..738e2c9bec30e25dae38b9f8ce083cec3fc70983 100644 >--- a/LayoutTests/fast/dom/css-set-property-exception-expected.txt >+++ b/LayoutTests/fast/dom/css-set-property-exception-expected.txt >@@ -1,16 +1,16 @@ > Test for bug 7296. > > This test checks to see whether you get exceptions when setting a property with a "bad value". Setting using JavaScript property syntax and with setProperty() should behave the same. > > It is OK if the order of properties changes from the expected results - IE 6 and Firefox 2 don't agree on it anyway. > > This is the test element. > >-Successfully set display to "block"; cssText is now: "top: 0px; display: block; bottom: 1px;". >+Successfully set display to "block"; cssText is now: "top: 0px; bottom: 1px; display: block;". > Successfully set display to "foobar"; cssText is now: "top: 0px; display: none; bottom: 1px;". > Successfully set display to ""; cssText is now: "top: 0px; bottom: 1px;". > Successfully set display to null; cssText is now: "top: 0px; bottom: 1px;". >-Successfully set display to "block" with setProperty; cssText is now: "top: 0px; display: block; bottom: 1px;". >+Successfully set display to "block" with setProperty; cssText is now: "top: 0px; bottom: 1px; display: block;". > Successfully set display to "foobar" with setProperty; cssText is now: "top: 0px; display: none; bottom: 1px;". > Successfully set display to "" with setProperty; cssText is now: "top: 0px; bottom: 1px;". > Successfully set display to null with setProperty; cssText is now: "top: 0px; bottom: 1px;". >diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001-expected.txt >index f327d4ac80b2a11d629765380382bde5c1abcff9..5eacaa76ef82ba261054e2468167cac239484745 100644 >--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001-expected.txt >+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-mutationrecord-001-expected.txt >@@ -1,5 +1,3 @@ > >-Harness Error (TIMEOUT), message = null >- >-NOTRUN CSSStyleDeclaration.setPropertyValue queues a mutation record, even if not mutated >+PASS CSSStyleDeclaration.setPropertyValue queues a mutation record, even if not mutated > >diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-setter-order-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-setter-order-expected.txt >index eb066fd1415408933d41e30424b68b794200b3c0..057696adbf58c102e8bb93b120c6ec6dc692137b 100644 >--- a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-setter-order-expected.txt >+++ b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-setter-order-expected.txt >@@ -1,6 +1,6 @@ > >-FAIL setProperty with existing longhand should change order assert_array_equals: Property order should match after setting existing property property 0, expected "bottom" but got "top" >-FAIL invoke property setter with existing longhand should change order assert_array_equals: Property order should match after setting existing property property 0, expected "bottom" but got "top" >-FAIL setProperty with existing shorthand should change order assert_array_equals: Property order should match after setting an existing shorthand property 0, expected "top" but got "margin-top" >-FAIL invoke property setter with existing shorthand should change order assert_array_equals: Property order should match after setting an existing shorthand property 0, expected "top" but got "margin-top" >+PASS setProperty with existing longhand should change order >+PASS invoke property setter with existing longhand should change order >+PASS setProperty with existing shorthand should change order >+PASS invoke property setter with existing shorthand should change order >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=340866">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 179391
:
340796
|
340797
|
340801
|
340844
|
340852
|
340853
|
340854
|
340856
|
340859
|
340866
|
340961
|
340965
|
340967
|
340971
|
340972