Nothing Special   »   [go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cssom] Serialization of CSS declaration block returned from getComputedStyle #1033

Closed
upsuper opened this issue Feb 14, 2017 · 19 comments · Fixed by #4945
Closed

[cssom] Serialization of CSS declaration block returned from getComputedStyle #1033

upsuper opened this issue Feb 14, 2017 · 19 comments · Fixed by #4945
Labels

Comments

@upsuper
Copy link
Member
upsuper commented Feb 14, 2017

How should CSS declaration block returned from getComputedStyle be serialized? It contains all longhand properties.

Currently, Gecko and Edge return empty string for it, while WebKit and Blink (probably inherit from WebKit) seem to return serialization of all longhand properties (i.e. not trying to generate shorthands). Apparently none follows the spec.

Following the same algorithm for cssText of declaration block from getComputedStyle is complicated, and probably isn't worth. So I suggest that the spec should either follow Gecko and Edge to return empty string, or follow WebKit and Blink to just return all longhand properties.

I would prefer we just make it return empty string... But there is a webcompat issue reported to Firefox for lack of its support: webcompat/web-bugs#3822. This is a Google website though, so maybe we can just ask Google to fix it and ignore this issue...

@upsuper upsuper added the cssom-1 Current Work label Feb 14, 2017
@mrego
Copy link
Member
mrego commented Feb 14, 2017

We've similar issues in CSS Grid Layout spec.
This was an old mail to www-style: https://lists.w3.org/Archives/Public/www-style/2015Jul/0242.html

A very basic example:

myGrid.style.grid = "100px / 50px";
console.log("style: " + myGrid.style.grid);
console.log("computedStyle: " + getComputedStyle(myGrid).grid);

The output right now in Chrome is:

style:
computedStyle: 100px / 50px / none / row / auto / auto / 0px / 0px

And in Firefox:

style: 100px / 50px;
computedStyle:

@upsuper
Copy link
Member Author
upsuper commented Feb 14, 2017

I don't think that's something related. IIRC in general we don't serialize new shorthands in computed style, only longhands are serialized there. There are several shorthands get serialized in computed style just for backward compatibility.

@mrego
Copy link
Member
mrego commented Feb 14, 2017

Oops, ok I messed it up, I thought you were talking about shorthands.

@FremyCompany
Copy link
Contributor
FremyCompany commented Feb 14, 2017

(Adding myself to the thread; but could you clarify what you think Chrome doesn't do per standards here? I have the impression their behavior is correct in that case at first glance)

(Just to clarify, talking about the initial issue, not the shorthands-in-gCS one)

@upsuper
Copy link
Member Author
upsuper commented Feb 14, 2017

could you clarify what you think Chrome doesn't do per standards here? I have the impression their behavior is correct in that case at first glance

In the spec, the serialize a CSS declaration block algorithm tries to construct shorthands whenever possible. In case of declaration block from getComputedStyle, since all longhands are available, most of shorthands would be constructible. However, Chrome doesn't seem to construct any shorthand in that case.

@tabatkins
Copy link
Member
tabatkins commented Feb 22, 2017

On the call today WG resolved to accept Firefox's and Edge's behavior, and make the object serialize to the empty string.

I'll file a bug on Chrome to fix this, and start evangelizing at Drive to fix the webcompat bug.

@FremyCompany
Copy link
Contributor

Thanks Tab!

@zcorpan
Copy link
Member
zcorpan commented Mar 6, 2017

@upsuper

There are several shorthands get serialized in computed style just for backward compatibility.

Which are these?

@upsuper
Copy link
Member Author
upsuper commented Mar 6, 2017

Mainly the shorthands which are previously longhands, e.g. background-position, text-decoration, overflow, and mask.

@zcorpan
Copy link
Member
zcorpan commented Mar 15, 2017

Given the following case, it seems browsers all have some logic to try to serialize as shorthands when possible. It's unclear to me why we want to avoid doing that for getComputedStyle specifically?

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4948

<!DOCTYPE html>
<p style="border:1em solid">
<p style="border:1em solid; border-right-width: 1px">
<p style="border:1em solid; border-right-width: 1px !important">
<script>
for (var p of document.querySelectorAll('p'))
  p.style.color = 'purple';
</script>

(check the value of the style attributes after the script has run.)

@emilio
Copy link
Collaborator
emilio commented Apr 27, 2018

@tabatkins Could you link it to the Chrome bug if you can find it please? I think this resolution makes sense, but Chrome hasn't updated its behavior yet :/.

I'll file a WebKit bug pointing to that bug as well.

@carlosame
Copy link

Like @zcorpan , I do not see for which reason computed styles cannot have a meaningful serialization. A computed style is still a style, after all.

I'm the developer of a CSS tool (a CSSOM library) that happens to behave like Chrome (although there is an optional method that builds the shorthands), and see no reason to change to returning an empty string (what's more, I have users depending on the current behaviour).

@upsuper
Copy link
Member Author
upsuper commented May 3, 2018

@zcorpan @carlosame

From the implementation side, computed style and specified style are usually stored in different forms in browsers, and when browsers are asked to serialize a computed value, they serialize it from its internal form directly (rather than, say, convert to specified value then serialize the latter), however, shorthand serialization algorithms are usually written to handle declaration block of specified values.

It means that to serialize declaration block from gCS per spec, browsers would need extra work either to implement all shorthand serialization algorithms on computed value as well, or create a declaration block containing every property in their specified value form, and then use the algorithm of specified declaration block to serialize them.

Doing such conversion is not impossible, though. Actually, at least in Gecko, we have such mechanism for animation and transition, because our cascading is based on specified value, but interpolation happens on computed value, so we need to "uncompute" value to cascade animation and transition correctly.

However, many properties are required to serialize differently for gCS, and the "uncompute" mechanism may not exactly match that requirement, so reusing it can potentially further complicate things.

From the use case side, I'd argue that getting a serialization of a declaration block containing every property with shorthand serialized properly is hardly the ideal way for anything.

If you just want to persist and restore all values, you should just iterate all properties in that block, that would almost certainly be faster than relying on browser to generate the serialization of the huge declaration block, then parsing this huge serialization again, as can be seen in the discussion of webcompat/web-bugs#3822.

Also, shorthand serialization are usually for human readability, but would anyone want to read a declaration block serialization containing hundreds of properties?

Given that this needs probably non-trivial extra work on implementations, while it doesn't seem to be very useful, we should probably just make it as simple as possible so that implementations can be interoperable on the behavior.

@carlosame
Copy link

however, shorthand serialization algorithms are usually written to handle declaration block of specified values.

Omit the shorthands like Chrome does, then. I'm asking for a meaningful serialization, and not necessarily one with shorthands. IMHO a gCS serialization with only longhands may make more sense than one with shorthands, given the nature of the potential use cases.

However, many properties are required to serialize differently for gCS, and the "uncompute" mechanism may not exactly match that requirement, so reusing it can potentially further complicate things.

But you already implement getProperyValue. All that you need is to add it to a buffer. Does not sound overly complex.

From the use case side, I'd argue that getting a serialization of a declaration block containing every property with shorthand serialized properly is hardly the ideal way for anything.

Agreed.

If you just want to persist and restore all values, you should just iterate all properties in that block, that would almost certainly be faster than relying on browser to generate the serialization of the huge declaration block, then parsing this huge serialization again

Persist and restore is just one use case. The fact that doing it that way can be inefficient does not mean that gCS serializations are useless.

Also, shorthand serialization are usually for human readability, but would anyone want to read a declaration block serialization containing hundreds of properties?

My serializations do not (generally) contain hundreds of properties. I do not include properties that take their initial values due to defaulting (and the count of properties that are cascaded or inherited is what I return in getLength(), and not the total amount of properties known to my library).

Given that this needs probably non-trivial extra work on implementations, while it doesn't seem to be very useful, we should probably just make it as simple as possible so that implementations can be interoperable on the behavior.

On UAs, that serialization is IMHO useful for debugging purposes, and for CSS tools (like mine) it can have other uses (although I do know that the WG is not very concerned about non-UA use cases).

I have use cases about putting the serialization in the style attribute of a DOM document and passing it to a rendering library that does not know how to compute styles, but can retrieve them from the attributes. It can also be useful to write tools that monitor changes in the styling of the elements in the DOM tree of a web page.

That is, on UAs gCS serialization can be useful, and on other type of implementations it is definitely useful.

And to me, "make it as simple as possible" means the Chrome behaviour (but I do not know whether it serializes all the properties or only the cascaded-or-inherited, as I mentioned).

@upsuper
Copy link
Member Author
upsuper commented May 3, 2018

but I do not know whether it serializes all the properties or only the cascaded-or-inherited, as I mentioned

Every element has value for every property, so it has to be listing all of them.

My serializations do not (generally) contain hundreds of properties. I do not include properties that take their initial values due to defaulting (and the count of properties that are cascaded or inherited is what I return in getLength(), and not the total amount of properties known to my library).

That is very different from what computed value means to be.

@carlosame
Copy link

Every element has value for every property, so it has to be listing all of them.

A computed style needs to be able to retrieve a value for each property (for example with getPropertyValue()), but it is unclear that the gCS serialization must include all of them.

We are discussing about what the serialization should be, and now leaping from "the empty string" to "it has to be listing all of them".

That is very different from what computed value means to be.

getLength() on a computed style becomes totally meaningless if you do not define it that way.

The basic idea is: if you need the computed value for a given property, use getPropertyValue() (I also provide getPropertyCSSValue()). But my gCS serialization behaves as described previously, and IMHO is very meaningful (and far better than the empty string).

@carlosame
Copy link

A small clarification to my previous comment: in my library, the getPropertyValue() in the computed styles returns a value for properties that aren't found in the item collection, if the library knows about its initial value. The item method only returns the properties that were cascaded or inherited.

That's what I meant with:

if you need the computed value for a given property, use getPropertyValue()

emilio added a commit to emilio/web-platform-tests that referenced this issue Apr 12, 2020
emilio added a commit that referenced this issue Apr 12, 2020
emilio added a commit to emilio/web-platform-tests that referenced this issue Apr 13, 2020
emilio added a commit to web-platform-tests/wpt that referenced this issue Apr 13, 2020
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Apr 16, 2020
…sText returns the empty string for computed style., a=testonly

Automatic update from web-platform-tests
[cssom] Test that CSSStyleDeclaration.cssText returns the empty string for computed style.

Test for w3c/csswg-drafts#1033 / w3c/csswg-drafts#4945.

--

wpt-commits: 3b7c757d4483ff89cce443b29a68b68c3defcd96
wpt-pr: 22877
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 16, 2020
…sText returns the empty string for computed style., a=testonly

Automatic update from web-platform-tests
[cssom] Test that CSSStyleDeclaration.cssText returns the empty string for computed style.

Test for w3c/csswg-drafts#1033 / w3c/csswg-drafts#4945.

--

wpt-commits: 3b7c757d4483ff89cce443b29a68b68c3defcd96
wpt-pr: 22877
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Apr 20, 2020
…sText returns the empty string for computed style., a=testonly

Automatic update from web-platform-tests
[cssom] Test that CSSStyleDeclaration.cssText returns the empty string for computed style.

Test for w3c/csswg-drafts#1033 / w3c/csswg-drafts#4945.

--

wpt-commits: 3b7c757d4483ff89cce443b29a68b68c3defcd96
wpt-pr: 22877

UltraBlame original commit: a305d1873e80a13e5528043bfe145be5ba7bde76
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Apr 20, 2020
…sText returns the empty string for computed style., a=testonly

Automatic update from web-platform-tests
[cssom] Test that CSSStyleDeclaration.cssText returns the empty string for computed style.

Test for w3c/csswg-drafts#1033 / w3c/csswg-drafts#4945.

--

wpt-commits: 3b7c757d4483ff89cce443b29a68b68c3defcd96
wpt-pr: 22877

UltraBlame original commit: a305d1873e80a13e5528043bfe145be5ba7bde76
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Apr 20, 2020
…sText returns the empty string for computed style., a=testonly

Automatic update from web-platform-tests
[cssom] Test that CSSStyleDeclaration.cssText returns the empty string for computed style.

Test for w3c/csswg-drafts#1033 / w3c/csswg-drafts#4945.

--

wpt-commits: 3b7c757d4483ff89cce443b29a68b68c3defcd96
wpt-pr: 22877

UltraBlame original commit: a305d1873e80a13e5528043bfe145be5ba7bde76
JTensai pushed a commit to JTensai/csswg-drafts that referenced this issue May 13, 2020
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 18, 2020
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 18, 2020
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2020
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2020
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Nov 19, 2020
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 26, 2020
…claration.cssText, a=testonly

Automatic update from web-platform-tests
[CSS] Return empty string for CSSStyleDeclaration.cssText

This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}

--

wpt-commits: 37a7ebd1a46e1bca71f1cda94922b681555519a3
wpt-pr: 26561
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Nov 27, 2020
…claration.cssText, a=testonly

Automatic update from web-platform-tests
[CSS] Return empty string for CSSStyleDeclaration.cssText

This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}

--

wpt-commits: 37a7ebd1a46e1bca71f1cda94922b681555519a3
wpt-pr: 26561
sylvestre pushed a commit to sylvestre/gecko-dev that referenced this issue Nov 28, 2020
…claration.cssText, a=testonly

Automatic update from web-platform-tests
[CSS] Return empty string for CSSStyleDeclaration.cssText

This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}

--

wpt-commits: 37a7ebd1a46e1bca71f1cda94922b681555519a3
wpt-pr: 26561
@karlcow
Copy link
Member
karlcow commented Aug 16, 2021

@graouts
Copy link
Contributor
graouts commented Nov 10, 2021

This will be fixed in WebKit with bug 232943.

webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Nov 10, 2021
https://bugs.webkit.org/show_bug.cgi?id=232943

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

* web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt:

Source/WebCore:

See w3c/csswg-drafts#1033. This was an annoying test to fail because the output
would require a rebaseline every time we'd change something visible in the computed style.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::cssText const):

LayoutTests:

Remove all platform-specific expectations for the WPT css/cssom/cssstyledeclaration-csstext.html since the
assertion that would fail differently on various platforms now passes everywhere.

* platform/gtk/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/wpe/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.



Canonical link: https://commits.webkit.org/244108@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@285604 268f45cc-cd09-0410-ab3c-d52691b4dbfc
annulen pushed a commit to qtwebkit/qtwebkit that referenced this issue Nov 11, 2021
https://bugs.webkit.org/show_bug.cgi?id=232943

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

* web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt:

Source/WebCore:

See w3c/csswg-drafts#1033. This was an annoying test to fail because the output
would require a rebaseline every time we'd change something visible in the computed style.

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::CSSComputedStyleDeclaration::cssText const):

LayoutTests:

Remove all platform-specific expectations for the WPT css/cssom/cssstyledeclaration-csstext.html since the
assertion that would fail differently on various platforms now passes everywhere.

* platform/gtk/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/ios-wk2/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/ios/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.
* platform/wpe/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-expected.txt: Removed.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@285604 268f45cc-cd09-0410-ab3c-d52691b4dbfc
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
This patch changes the code to return empty string for
CSSStyleDeclaration.cssText.

Discussion at:
w3c/csswg-drafts#1033

Makes tests pass:
external/wpt/css/cssom/cssstyledeclaration-csstext.html

Bug: 1146888
Change-Id: Ibca415b7606d2cc5110ca633c87a77fac3e74ca8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546305
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Jaeyong Bae <jdragon.bae@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829173}
GitOrigin-RevId: 07899db4893ca0e990e5563ec15832e85d258175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
11 participants