Bug 215646 - The initial value of quotes should be auto.
Summary: The initial value of quotes should be auto.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emilio Cobos Álvarez (:emilio)
URL:
Keywords: InRadar
Depends on: 215751
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-19 05:43 PDT by Emilio Cobos Álvarez (:emilio)
Modified: 2020-08-24 12:18 PDT (History)
13 users (show)

See Also:


Attachments
Patch (169.11 KB, patch)
2020-08-22 06:51 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff
Patch (8.11 KB, patch)
2020-08-22 06:56 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff
Patch (8.08 KB, patch)
2020-08-22 17:55 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff
Patch (8.10 KB, patch)
2020-08-22 19:29 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff
Patch (26.82 KB, patch)
2020-08-23 10:45 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff
Patch (85.43 KB, patch)
2020-08-23 12:07 PDT, Emilio Cobos Álvarez (:emilio)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emilio Cobos Álvarez (:emilio) 2020-08-19 05:43:14 PDT
Right now the quotes property returns an empty string when getting it from getComputedStyle on WebKit-based browsers. That makes no sense (that's not a valid property value of course).

See https://github.com/w3c/csswg-drafts/issues/4074 for the CSSWG resolution to name this value "auto". Firefox implements this since ages ago, and there are WPTs for this.
Comment 1 Emilio Cobos Álvarez (:emilio) 2020-08-22 06:51:48 PDT
Created attachment 407049 [details]
Patch
Comment 2 Emilio Cobos Álvarez (:emilio) 2020-08-22 06:56:19 PDT
Created attachment 407051 [details]
Patch
Comment 3 Darin Adler 2020-08-22 14:10:40 PDT
Comment on attachment 407051 [details]
Patch

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

Patch looks good, and I will review after we have the tests uploaded and the EWS applies and confirms things are working.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:477
> +static Ref<CSSValue>valueForQuotes(const QuotesData* quotes)

Missing space after ">".
Comment 4 Emilio Cobos Álvarez (:emilio) 2020-08-22 17:55:48 PDT
Created attachment 407060 [details]
Patch
Comment 5 Emilio Cobos Álvarez (:emilio) 2020-08-22 18:09:17 PDT
(In reply to Darin Adler from comment #3)
> Missing space after ">".

Thanks, done :)
Comment 6 Emilio Cobos Álvarez (:emilio) 2020-08-22 19:29:19 PDT
Created attachment 407061 [details]
Patch
Comment 7 Emilio Cobos Álvarez (:emilio) 2020-08-23 10:45:14 PDT
Created attachment 407075 [details]
Patch
Comment 8 Darin Adler 2020-08-23 11:27:59 PDT
Comment on attachment 407075 [details]
Patch

I did a grep, and all of these files have "quotes: ;" in them that have to be changed to "quotes: auto;"

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

So the patch needs to include changes to all 4 of the files. And they need to be mentioned in the change log.
Comment 9 Emilio Cobos Álvarez (:emilio) 2020-08-23 12:00:35 PDT
Gah, thanks, I did update that test but didn't realize that the baselines were platform specific.
Comment 10 Emilio Cobos Álvarez (:emilio) 2020-08-23 12:07:54 PDT
Created attachment 407077 [details]
Patch
Comment 11 Darin Adler 2020-08-23 12:33:41 PDT
(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)
> Gah, thanks, I did update that test but didn't realize that the baselines
> were platform specific.

I am not sure why those platform specific results have to exist. Might be removable with a little research. But obviously easiest to just update them in the short term.

Will review as soon as we get green EWS results.
Comment 12 Emilio Cobos Álvarez (:emilio) 2020-08-24 02:38:32 PDT
(In reply to Darin Adler from comment #11)
> (In reply to Emilio Cobos Álvarez (:emilio) from comment #9)
> I am not sure why those platform specific results have to exist. Might be
> removable with a little research. But obviously easiest to just update them
> in the short term.

I guess there are different properties enabled on different ports, and this test accidentally lists all supported CSS properties. The good thing though is that EWS seems green now :)
Comment 13 EWS 2020-08-24 12:17:57 PDT
Committed r266082: <https://trac.webkit.org/changeset/266082>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407077 [details].
Comment 14 Radar WebKit Bug Importer 2020-08-24 12:18:17 PDT
<rdar://problem/67692693>