Make CSSOM APIs append rather than replace slots in the declaration block. |
||||||||||
Issue descriptionOtherwise logical properties work inconsistently with them. See https://github.com/w3c/csswg-drafts/issues/1898 for more background and the CSSWG resolution.
,
Nov 8 2017
,
Nov 28 2017
Assuming there is a way to let us optimize for redundant setters - this should be a good bug for someone to take on. Here is the code: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/StylePropertySet.cpp?gsn=SetProperty&l=347 for the change.
,
Dec 6 2017
,
May 15 2018
,
May 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9d45b94d610d27094ccfd07d20b7283813fa0bf6 commit 9d45b94d610d27094ccfd07d20b7283813fa0bf6 Author: Emilio Cobos Álvarez <emilio@chromium.org> Date: Sat May 19 22:04:07 2018 [cssom] Make CSSOM APIs always append to the declaration block. ...instead of replacing if the property is already set. This matches the spec[1] which was changed as a resolution from[2]. Intent thread: https://groups.google.com/a/chromium.org/d/msg/blink-dev/lzBoa1EAQpI/strfpNczAwAJ [1]: https://drafts.csswg.org/cssom/#set-a-css-declaration [2]: https://github.com/w3c/csswg-drafts/issues/1898 Bug: 782407 Change-Id: I4b72c0207a4cf734fc2e415bd62c7863103ec309 Reviewed-on: https://chromium-review.googlesource.com/1054867 Commit-Queue: Emilio Cobos Álvarez <emilio@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#560180} [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/editing/pasteboard/dragstart-contains-default-content-expected.txt [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt [delete] https://crrev.com/ec82e04d6e4c45f133807224142330d441f4c54f/third_party/WebKit/LayoutTests/external/wpt/css/cssom/cssstyledeclaration-setter-order-expected.txt [delete] https://crrev.com/ec82e04d6e4c45f133807224142330d441f4c54f/third_party/WebKit/LayoutTests/fast/css/no-recalc-on-no-op-inline-style-changes.html [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes.html [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/fast/dom/css-set-property-exception-expected.txt [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/WebKit/LayoutTests/fast/events/ondrop-text-html-expected.txt [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/css_property_value_set.cc [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/css_property_value_set.h [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/dom_window_css.cc [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/parser/css_parser.cc [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/parser/css_parser.h [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/parser/css_parser_impl.cc [modify] https://crrev.com/9d45b94d610d27094ccfd07d20b7283813fa0bf6/third_party/blink/renderer/core/css/parser/css_parser_impl.h
,
May 20 2018
,
May 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f commit 710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f Author: Emilio Cobos Álvarez <emilio@chromium.org> Date: Mon May 21 16:11:46 2018 Cleanup no longer used argument in MutableCSSPropertyValueSet::SetProperty. I forgot to do this in the previous patch for issue 782407. Looks like the only caller that passed it really wants the new behavior, plus before the original patch for issue 782407 it was already kinda broken with custom properties... No new tests, since there's no behavior change. Bug: 782407 Change-Id: I410ed1481256b18886c5804f4cbecde9846f2a26 Reviewed-on: https://chromium-review.googlesource.com/1066062 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Emilio Cobos Álvarez <emilio@chromium.org> Cr-Commit-Position: refs/heads/master@{#560279} [modify] https://crrev.com/710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f/third_party/blink/renderer/core/css/css_property_value_set.cc [modify] https://crrev.com/710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f/third_party/blink/renderer/core/css/css_property_value_set.h
,
May 23 2018
,
Jun 7 2018
Patch was reverted in https://chromium.googlesource.com/chromium/src/+/85f8362c2b95986a4d7e574628ee8a4db33d62c1
,
Jun 7 2018
The change in #6 is also the probable cause of issue 845404 where tap gestures stopped triggering Contextual Search on Google search-result pages. Thanks for the revert -- now that issue stopped happening. The tap on plain text was causing a CSS change (some element was changing css to display:none), which prevented Contextual Search from triggering. If you'd like to know more details let me know. Emilio, I'll cc you on that bug. Please reach out to me (donnd@chromium.org) when attempting to reland this so we can check if this is still a problem. Thanks!
,
Jun 7 2018
Hi! Could I get some more information (either via here or privately) about what the page was trying to do? Was it observing / reacting to attribute mutations via MutationObserver? How did it detect the change in the page? Thank you!
,
Jun 7 2018
,
Jun 7 2018
Emilio, please request to merge the revert in #10 into M-68. This is blocking issue 845404.
,
Jun 7 2018
Emilio, in short the GestureManager checks if CSS styles change in response to a tap gesture before notifying Contextual Search here: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/input/gesture_manager.cc?type=cs&q=gesture_manager+style_changed&sq=package:chromium&g=0&l=309. I'm not sure what part of the SERP was actually changing -- it took a while to debug and I saw a lot of shifting behavior (as you can see in that bug). Please IM me so I can get your LDAP and so we can discuss in more detail. Thanks!
,
Jun 7 2018
Re. #14: There's a merge request for the revert in issue 845944 . Re. #15: I see, looks like that check doesn't really want to check StyleVersion(), but whether any styles have actually changed... I'm not a googler, so I don't have LDAP I presume :/. In any case, the reason for the regression and the shifting behavior is because StyleVersion() gets incremented even if no style in the page actually has changed (only if it could have). This change made StyleVersion() to get incremented more often.
,
Jun 7 2018
There's no guarantee that any computed styles changed with increasing StyleVersion(). |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by shend@chromium.org
, Nov 8 2017