Bug 155874

Summary: Align quirky number parsing with other browsers
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: CSSAssignee: Ali Juma <ajuma>
Status: RESOLVED FIXED    
Severity: Normal CC: ajuma, commit-queue, dino, eoconnor, hyatt, koivisto, simon.fraser, webkit-bug-importer, zcorpan
Priority: P2 Keywords: InRadar, WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 155870    
Attachments:
Description Flags
Patch
none
Patch none

Simon Fraser (smfr)
Reported 2016-03-24 22:55:57 PDT
We currently parse unit-less, non-zero numbers as valid time values, which breaks things like animation shorthand parsing (bug 155870). We should align with other browsers.
Attachments
Patch (4.65 KB, patch)
2017-07-18 16:20 PDT, Ali Juma 		no flags
DetailsFormatted DiffDiff
Patch (4.65 KB, patch)
2017-07-18 16:26 PDT, Ali Juma 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-03-24 22:56:37 PDT
<rdar://problem/25354495>
Simon Fraser (smfr)
Comment 2 2016-03-24 22:58:48 PDT
Here's the current code: inline bool CSSParser::shouldAcceptUnitLessValues(CSSParserValue& value, Units unitFlags, CSSParserMode cssParserMode) { // Qirks mode and svg presentation attributes accept unit less values. return (unitFlags & (FLength | FAngle | FTime)) && (!value.fValue || cssParserMode == CSSQuirksMode || cssParserMode == SVGAttributeMode); }
Simon Fraser (smfr)
Comment 3 2016-04-09 11:00:01 PDT
https://quirks.spec.whatwg.org/#the-unitless-length-quirk
Ali Juma
Comment 4 2017-07-18 13:52:01 PDT
Currently, the only properties this affects are transition-duration, animation-duration, and -webkit-marquee-speed. -webkit-marquee-speed is a bit of a special case though, since the marquee element's scrolldelay attribute (which is unit-less) gets converted to -webkit-marquee-speed using the CSS parser (in quirks mode, even when the document itself is not in quirks mode). It's also not supported by any other browser (Blink removed this property a while ago), so there's no concern about aligning with other browsers. So I'd be inclined to fix transition-duration and animation-duration, and leave -webkit-marquee-speed alone.
Simon Fraser (smfr)
Comment 5 2017-07-18 14:21:34 PDT
I think it's fine to treat -webkit-marquee-speed as not web-exposed, and just fix them all if it's easier.
Ali Juma
Comment 6 2017-07-18 16:20:14 PDT
Created attachment 315853 [details] Patch
Ali Juma
Comment 7 2017-07-18 16:23:44 PDT
(In reply to Simon Fraser (smfr) from comment #5) > I think it's fine to treat -webkit-marquee-speed as not web-exposed, and > just fix them all if it's easier. Leaving -webkit-marquee-speed as-is turned out to be easier, since fixing that would have required making HTMLMarqueeElement::collectStyleForPresentationAttribute append a unit to the value of the scrolldelay attribute before it gets converted to a -webkit-marquee-speed value.
Ali Juma
Comment 8 2017-07-18 16:26:42 PDT
Created attachment 315855 [details] Patch
WebKit Commit Bot
Comment 9 2017-07-18 16:54:48 PDT
Comment on attachment 315855 [details] Patch Clearing flags on attachment: 315855 Committed r219642: <http://trac.webkit.org/changeset/219642>
WebKit Commit Bot
Comment 10 2017-07-18 16:54:49 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.