Bug 95848

Summary: [css3-text] Add parsing support for -webkit-text-decoration-skip
Product: WebKit Reporter: Bruno Abinader (history only) <bruno.abinader>
Component: CSSAssignee: Bruno Abinader (history only) <bruno.abinader>
Status: ASSIGNED ---    
Severity: Normal CC: abecsi, cmarcelo, dbates, dglazkov, eric, fantasai.bugs, gtk-ews, gustavo, gyuyoung.kim, jchaffraix, lamarque, macpherson, menard, ojan.autocc, philn, rakuco, syoichi, vestbo, webkit.review.bot, xan.lopez
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dev.w3.org/csswg/css-text-decor-3/#text-decoration-skip-property
Bug Depends on:    
Bug Blocks: 58491, 92801, 95856    
Attachments:
Description Flags
Patch
none
Patch (EWS run only)
none
Patch
none
Patch (EWS only)
none
Patch
none
Patch (EWS only)
none
Patch none

Description Bruno Abinader (history only) 2012-09-05 06:05:05 PDT
This bug intends to add parsing support for "text-decoration-skip" CSS3 property. Rendering support is going to be implemented on another bug. This is a sub-task for bug 92801.
Comment 1 Bruno Abinader (history only) 2012-09-05 08:03:49 PDT
Created attachment 162252 [details]
Patch

Proposed patch.
Comment 2 Bruno Abinader (history only) 2012-09-05 08:10:13 PDT
Created attachment 162254 [details]
Patch (EWS run only)
Comment 3 Bruno Abinader (history only) 2012-10-23 06:22:46 PDT
Created attachment 170142 [details]
Patch

Updates on namespace (now CSS3_TEXT), layout test directories and StyleBuilder handler interface.
Comment 4 Bruno Abinader (history only) 2012-10-23 06:33:26 PDT
Created attachment 170145 [details]
Patch (EWS only)
Comment 5 Build Bot 2012-10-23 07:08:01 PDT
Comment on attachment 170145 [details]
Patch (EWS only)

Attachment 170145 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14483956
Comment 6 Build Bot 2012-10-23 07:08:58 PDT
Comment on attachment 170145 [details]
Patch (EWS only)

Attachment 170145 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14491877
Comment 7 kov's GTK+ EWS bot 2012-10-23 09:12:28 PDT
Comment on attachment 170145 [details]
Patch (EWS only)

Attachment 170145 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14524022
Comment 8 WebKit Review Bot 2012-10-23 11:11:04 PDT
Comment on attachment 170145 [details]
Patch (EWS only)

Attachment 170145 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14500616

New failing tests:
fast/css3-text/css3-text-decoration/text-decoration-style.html
fast/css3-text/css3-text-decoration/repaint/repaint-text-decoration-style.html
fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-decoration-skip.html
Comment 9 Bruno Abinader (history only) 2012-10-26 07:40:19 PDT
Created attachment 170925 [details]
Patch

Fixed layout test internal paths after directory changes.
Comment 10 Bruno Abinader (history only) 2012-10-26 11:41:54 PDT
Created attachment 170973 [details]
Patch (EWS only)

More info on failing layout tests in bug 100546 "[css3-text] Provide pixel results for text decoration style layout tests".
Comment 11 Build Bot 2012-10-26 12:12:16 PDT
Comment on attachment 170973 [details]
Patch (EWS only)

Attachment 170973 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14593766
Comment 12 WebKit Review Bot 2012-10-26 13:38:14 PDT
Comment on attachment 170973 [details]
Patch (EWS only)

Attachment 170973 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14593790

New failing tests:
fast/css3-text/css3-text-decoration/text-decoration-style.html
fast/css3-text/css3-text-decoration/repaint/repaint-text-decoration-style.html
Comment 13 kov's GTK+ EWS bot 2012-10-26 14:30:54 PDT
Comment on attachment 170973 [details]
Patch (EWS only)

Attachment 170973 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14612130
Comment 14 Bruno Abinader (history only) 2012-12-12 09:47:24 PST
Created attachment 179069 [details]
Patch

Rebased after text-align-last changes.
Comment 15 fantasai 2013-03-05 23:44:06 PST
Please don't land any parsing support for properties for which there is no actual support. This is counter to the way CSS parsing is supposed to work.
  http://www.w3.org/TR/CSS/#partial
Comment 16 Bruno Abinader (history only) 2013-03-06 04:59:33 PST
(In reply to comment #15)
> Please don't land any parsing support for properties for which there is no actual support. This is counter to the way CSS parsing is supposed to work.
>   http://www.w3.org/TR/CSS/#partial

Rendering support should be handled in bug 95856 (as refered in "blocks" section), though there's no actually implementation yet. Unfortunately I do not have resources to implement it, so indeed, the patch proposed on this bug shall not land before there's a corresponding implementation on bug 95856.
Comment 17 Julien Chaffraix 2013-03-06 10:08:58 PST
(In reply to comment #15)
> Please don't land any parsing support for properties for which there is no actual support. This is counter to the way CSS parsing is supposed to work.
>   http://www.w3.org/TR/CSS/#partial

You are missing the point here. All new code is landed behind a compile flag that is *disabled* by default (ie not exposed to the web). So yes, we are breaking feature detection but on a feature that is not shipped. We also make sure we don't toggle the flag until all the bits are done.

AFAICT this is matching what you said and I like this practice because it enables better quality reviews by splitting massive patches into manageable pieces.
Comment 18 Anders Carlsson 2014-02-05 11:02:31 PST
Comment on attachment 179069 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.