Bug 88228

Summary: [CSS3 Backgrounds and Borders] Implement box-decoration-break rendering.
Product: WebKit Reporter: Alexis Menard (darktears) <menard>
Component: CSSAssignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, eric, hyatt, jchaffraix, jonlee, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 27576    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alexis Menard (darktears) 2012-06-04 05:28:44 PDT 
[CSS3 Backgrounds and Borders] Implement box-decoration-break rendering.
Comment 1 Alexis Menard (darktears) 2012-06-04 05:32:13 PDT 
Created attachment 145567 [details]
Patch
Comment 2 Alexis Menard (darktears) 2012-06-04 05:33:50 PDT 
(In reply to comment #1)
> Created an attachment (id=145567) [details]
> Patch

I believe I don't cover the entire feature but let's begin the feedback round. So far the layout test is the only real test-case I found and appear to be what Opera has implemented in their latest version. I'm very open to extend the coverage and also implement the missing part of that patch.
Comment 3 Alexis Menard (darktears) 2012-06-04 08:41:11 PDT 
Created attachment 145592 [details]
Patch
Comment 4 Alexis Menard (darktears) 2012-06-13 04:28:07 PDT 
Created attachment 147287 [details]
Patch
Comment 5 Eric Seidel (no email) 2012-06-20 10:39:04 PDT 
Comment on attachment 147287 [details]
Patch

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

Looks totally fine to me.

> Source/WebCore/rendering/InlineFlowBox.cpp:1119
> +        paintInfo.context->clip(LayoutRect(rect.x(), rect.y(), width(), height()));

Sad you have to create your own LayoutRect like this.  SUrprised there isn't a helper.
Comment 6 WebKit Review Bot 2012-06-20 10:49:39 PDT 
Comment on attachment 147287 [details]
Patch

Clearing flags on attachment: 147287

Committed r120835: <http://trac.webkit.org/changeset/120835>
Comment 7 WebKit Review Bot 2012-06-20 10:49:50 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Beth Dakin 2012-06-20 16:32:27 PDT 
It looks like this test is failing on the Mac bots.
Comment 9 Jon Lee 2012-06-21 10:31:08 PDT 
Skipped on Mac. See bug 89620.
Comment 10 Alexis Menard (darktears) 2012-06-21 10:34:31 PDT 
(In reply to comment #9)
> Skipped on Mac. See bug 89620.

Ok, sorry about that. On the moment I do not have a Mac machine next to me, but will have next week so I will investigate. I thought a ref test would protect me a bit from the divergence between platforms but it seems not in this case.