Bug 232932 – Default computed value for "content" should be "none" for ::before and ::after

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antoine Quint]

Default computed value for "content" should be "none" for ::before and ::after

[Antoine Quint]

Created attachment 443787 [details]
Patch

[Antoine Quint]

Created attachment 443813 [details]
Patch

[Antoine Quint]

Created attachment 443846 [details]
Patch

[Darin Adler]

Comment on attachment 443846 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443846&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1822
> +        list->append(cssValuePool.createIdentifierValue(isBeforeOrAfter ? CSSValueNone : CSSValueNormal));

Is it correct that we return this in a list rather than just the identifier, not in a list? Seems to me that it should not be a list, just a single value.

Are we testing any code path that can tell the two apart, or only tests that involve serializing as text where we can’t tell a list with one element apart from a value?

[Antoine Quint]

(In reply to Darin Adler from comment #4)
> Comment on attachment 443846 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443846&action=review
> 
> > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1822
> > +        list->append(cssValuePool.createIdentifierValue(isBeforeOrAfter ? CSSValueNone : CSSValueNormal));
> 
> Is it correct that we return this in a list rather than just the identifier,
> not in a list? Seems to me that it should not be a list, just a single value.

I assume as much as we return all values, even single ones, in a list as well. For instance, `content: "test"` will return that single value in a list.

> Are we testing any code path that can tell the two apart, or only tests that
> involve serializing as text where we can’t tell a list with one element
> apart from a value?

I haven't looked at all the WPT coverage for `content` to answer this, but the particular tests that are progressing only look at the serialized text value.

[Antoine Quint]

I will land this as-is because returning a single value within a list is not a new behavior here.

[EWS]

Committed r285621 (244122@main): <https://commits.webkit.org/244122@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443846 [details].

[Radar WebKit Bug Importer]

<rdar://problem/85287828>

[Darin Adler]

Comment on attachment 443846 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443846&action=review

>>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1822
>>> +        list->append(cssValuePool.createIdentifierValue(isBeforeOrAfter ? CSSValueNone : CSSValueNormal));
>> 
>> Is it correct that we return this in a list rather than just the identifier, not in a list? Seems to me that it should not be a list, just a single value.
>> 
>> Are we testing any code path that can tell the two apart, or only tests that involve serializing as text where we can’t tell a list with one element apart from a value?
> 
> I assume as much as we return all values, even single ones, in a list as well. For instance, `content: "test"` will return that single value in a list.

This patch is fine.

Separately, I think that we need to add tests that can tell the difference between lists and single values to WPT. Otherwise, differences between browsers can crop up and cause website incompatibilities.