Bug 91951

Summary: CSS3 calc: unprefix implementation
Product: WebKit Reporter: Mike Lawther <mikelawther>
Component: CSSAssignee: Mike Lawther <mikelawther>
Status: RESOLVED FIXED    
Severity: Normal CC: 7raivis, cmarcelo, ericbidelman, kling, koivisto, macpherson, mathias, menard, ojan.autocc, ojan, paulirish, rniwa, shikolay, simon.fraser, syoichi, tony, webkit.bugzilla, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 16662    
Attachments:
Description Flags
Patch none

Description Mike Lawther 2012-07-22 16:44:11 PDT
IE9 and Mozilla (https://bugzilla.mozilla.org/show_bug.cgi?id=771678) have unprefixed their calc implementations.

Normally we'd wait for the spec (http://www.w3.org/TR/css3-values/) to to to Candidate Rec (CR). It's currently at Last Call, and comments were due 29/03/2012.  

With 2 unprefixed implementations, it's enough to check that ours is compatible with theirs.

IE calc tests: http://samples.msdn.microsoft.com/ietestcenter/#css3valuesandunits
Moz calc tests: http://mxr.mozilla.org/mozilla-central/source/layout/reftests/css-calc/
Comment 1 Mike Lawther 2013-01-17 22:31:29 PST
I've tested -webkit-calc() against both the IE and Mozilla test suites.

IE : PASS
Moz : PASS everything except:
  - text-indent-intrinsic-1.html : this is a problem in WebKit's text-indent intrinsic
  - width-table-auto-1.html, width-table-fixed-1.html : both table related, and per spec it is acceptable to ignore calc() in tables.

The spec (http://www.w3.org/TR/css3-values/) is now at Candidate Rec.

With the spec at CR, two shipping unprefixed implementations, and broad compat between ours and theirs, I reckon it's OK to go ahead with the unprefixing.
Comment 2 Binyamin 2013-01-19 22:39:22 PST
Just awaiting for unprefix.
Comment 3 Mike Lawther 2013-01-20 20:59:12 PST
Created attachment 183701 [details]
Patch
Comment 4 Mike Lawther 2013-01-20 21:07:59 PST
Testing-wise, I've replaced all -webkit-calc() in the tests with calc(), as the unprefixed version is what we'll support from now on. I've kept a copy of the most basic test using -webkit-calc() to ensure our support for it doesn't completely break.

This seemed a sane approach - keeping an unprefixed copy of the entire test directory seemed like overkill.
Comment 5 Ryosuke Niwa 2013-01-20 21:15:30 PST
(In reply to comment #1)
> Moz : PASS everything except:
>   - text-indent-intrinsic-1.html : this is a problem in WebKit's text-indent intrinsic
>   - width-table-auto-1.html, width-table-fixed-1.html : both table related, and per spec it is acceptable to ignore calc() in tables.

Please file bugs to track these failures.
Comment 6 Ryosuke Niwa 2013-01-20 21:16:35 PST
Did you try the Mozilla tests after fixes are landed in https://bugzilla.mozilla.org/show_bug.cgi?id=826582 ?
Comment 7 Mike Lawther 2013-01-20 22:42:48 PST
Text indent bug filed as https://bugs.webkit.org/show_bug.cgi?id=107416. 

wrt filing bugs about the table differences - both our impls are to spec. All the bug would say is 'this behaviour is different'?

I did not run the tests after Moz upstreamed them to the W3C. I can do this and report back.
Comment 8 Ojan Vafai 2013-01-20 22:53:36 PST
Comment on attachment 183701 [details]
Patch

<3
Comment 9 Ryosuke Niwa 2013-01-20 22:54:51 PST
(In reply to comment #7)
> wrt filing bugs about the table differences - both our impls are to spec. All the bug would say is 'this behaviour is different'?

That would be great. It'll helps us document the issue at least. If anything, we should check if there's any compatibility concerns to pick one behavior over another.
Comment 10 WebKit Review Bot 2013-01-20 23:06:45 PST
Comment on attachment 183701 [details]
Patch

Clearing flags on attachment: 183701

Committed r140300: <http://trac.webkit.org/changeset/140300>
Comment 11 WebKit Review Bot 2013-01-20 23:06:51 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Mike Lawther 2013-01-21 15:51:50 PST
As requested, I tested Mozilla's updated tests from http://mxr.mozilla.org/mozilla-central/source/layout/reftests/w3c-css/submitted/values3/, and the results were the same as I reported in comment #1.

Aside - now that we are both unprefixed, I was able to do this testing without modifying any test source code - yay for compat!
Comment 13 Ryosuke Niwa 2013-01-21 15:57:33 PST
(In reply to comment #12)
> As requested, I tested Mozilla's updated tests from http://mxr.mozilla.org/mozilla-central/source/layout/reftests/w3c-css/submitted/values3/, and the results were the same as I reported in comment #1.
> 
> Aside - now that we are both unprefixed, I was able to do this testing without modifying any test source code - yay for compat!

Thanks for the follow up!
Comment 14 Mike Lawther 2013-01-21 16:04:57 PST
Table differences filed as https://bugs.webkit.org/show_bug.cgi?id=107482