| Summary: | wpt/css/css-images/gradient/color-stops-parsing.html fails | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
| Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | clopez, commit-queue, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, macpherson, menard, sabouhallawa, simon.fraser, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Simon Fraser (smfr)
2019-07-28 15:54:28 PDT
It fails on cases where there are multiple length values not separated by commas: linear-gradient(black, 25% 50%, white) Chrome and Firefox pass these tests. The relevant parsing code in Chrome is protected by RuntimeEnabledFeatures::MultipleColorStopPositionsEnabled() which was added via https://chromiumcodereview.appspot.com/2799793002/. I'm not sure if this is enabled by default in Chrome yet. This was shipped in Chrome m71: https://chromium-review.googlesource.com/c/chromium/src/+/1230018/ (see also https://chromestatus.com/feature/5712111258828800) Created attachment 375050 [details]
Patch
Comment on attachment 375050 [details]
Patch
This breaks a conic gradient test.
Created attachment 381563 [details]
Patch
Created attachment 381601 [details]
Patch
Comment on attachment 381601 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381601&action=review > Source/WebCore/css/CSSGradientValue.cpp:123 > + else if (i) { Is it possible that i == 0 && stop.isMidpoint == true? I think it should not. If this is correct then I would suggest replacing else if (i) by ASSERT(i). (In reply to Said Abou-Hallawa from comment #7) > Comment on attachment 381601 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381601&action=review > > > Source/WebCore/css/CSSGradientValue.cpp:123 > > + else if (i) { > > Is it possible that i == 0 && stop.isMidpoint == true? I think it should > not. If this is correct then I would suggest replacing else if (i) by > ASSERT(i). I don't think you can get a mid-point stop before a colorless stop. I could add the assertion. |