Bug 189142

Summary: Properties that take <position> should not accept 3 values
Product: WebKit Reporter: Eric Willigers <ericwilligers>
Component: CSSAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, koivisto, macpherson, menard, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: FromImplementor, InRadar
Version: Safari 11   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch koivisto: review+

Description Eric Willigers 2018-08-29 20:14:52 PDT
https://drafts.csswg.org/css-images-3/#the-object-position
The relevant syntax is

<position> = [
  [ left | center | right ] || [ top | center | bottom ]
|
  [ left | center | right | <length-percentage> ]
  [ top | center | bottom | <length-percentage> ]?
|
  [ [ left | right ] <length-percentage> ] &&
  [ [ top | bottom ] <length-percentage> ]
]

A number of test cases currently fail:
https://wpt.fyi/results/css/css-images/parsing/object-position-invalid.html

e.g. "center left 1px" should not be accepted.

This also applies to <position> in gradients
Comment 1 Radar WebKit Bug Importer 2018-09-04 13:49:19 PDT
<rdar://problem/44110851>
Comment 2 Simon Fraser (smfr) 2019-10-25 18:26:32 PDT
Seems like some css/css-shapes tests are still expecting the 3-value syntax.
Comment 3 Simon Fraser (smfr) 2019-10-25 18:30:56 PDT
Created attachment 381985 [details]
Patch
Comment 4 Simon Fraser (smfr) 2019-10-25 18:37:21 PDT
*** Bug 200213 has been marked as a duplicate of this bug. ***
Comment 5 Simon Fraser (smfr) 2019-10-25 19:47:25 PDT
Created attachment 381988 [details]
Patch
Comment 6 Antti Koivisto 2019-10-28 11:02:43 PDT
Comment on attachment 381988 [details]
Patch

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

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:926
> +        if (values[i + 1] && !values[i + 1]->isValueID())

Relying on caller to ensure i + 1 is not out of bounds is not very nice. How about passing values as std::array or something?
Comment 7 Simon Fraser (smfr) 2019-10-28 13:32:34 PDT
https://trac.webkit.org/r251668
Comment 8 Simon Fraser (smfr) 2019-11-22 15:00:12 PST
*** Bug 148211 has been marked as a duplicate of this bug. ***