Summary: | The initial value of quotes should be auto. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emilio Cobos Álvarez (:emilio) <emilio> | ||||||||||||||
Component: | CSS | Assignee: | Emilio Cobos Álvarez (:emilio) <emilio> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | changseok, clopez, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 215751 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Emilio Cobos Álvarez (:emilio)
2020-08-19 05:43:14 PDT
Created attachment 407049 [details]
Patch
Created attachment 407051 [details]
Patch
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 ">". Created attachment 407060 [details]
Patch
(In reply to Darin Adler from comment #3) > Missing space after ">". Thanks, done :) Created attachment 407061 [details]
Patch
Created attachment 407075 [details]
Patch
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.
Gah, thanks, I did update that test but didn't realize that the baselines were platform specific. Created attachment 407077 [details]
Patch
(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. (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 :) Committed r266082: <https://trac.webkit.org/changeset/266082> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407077 [details]. |