Bug 87678 – [CSS3 Backgrounds and Borders] Implement box-decoration-break parsing.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alexis Menard (darktears)]

[CSS3 Backgrounds and Borders] Implement box-decoration-break parsing.

[Alexis Menard (darktears)]

Created attachment 144394 [details]
Patch

[Alexis Menard (darktears)]

(In reply to comment #1)
> Created an attachment (id=144394) [details]
> Patch

Let's give a shot, I'm not yet familiar with all the bits and pieces in the rendering code so please forgive my ignorance if I misplaced the bits of box-decoration-break.

[Eric Seidel (no email)]

This technically looks fine.  Are we supposed to be adding -webkit- these days or not?

[Alexis Menard (darktears)]

(In reply to comment #3)
> This technically looks fine.  Are we supposed to be adding -webkit- these days or not?

Good question. I can easily add the -webkit counterpart if needed. Not sure at the end what was the conclusion on www-style.

[Build Bot]

Comment on attachment 144394 [details]
Patch

Attachment 144394 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12853048

[Alexis Menard (darktears)]

Created attachment 144396 [details]
Patch

[WebKit Review Bot]

Comment on attachment 144396 [details]
Patch

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

New failing tests:
fast/css/getComputedStyle/computed-style-without-renderer.html
fast/css/getComputedStyle/computed-style.html
svg/css/getComputedStyle-basic.xhtml

[WebKit Review Bot]

Created attachment 144403 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Alexis Menard (darktears)]

Created attachment 144406 [details]
Patch

[WebKit Review Bot]

Comment on attachment 144406 [details]
Patch

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

New failing tests:
fast/css/getComputedStyle/computed-style-without-renderer.html

[WebKit Review Bot]

Created attachment 144412 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 144406 [details]
Patch

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

New failing tests:
fast/css/getComputedStyle/computed-style-without-renderer.html

[WebKit Review Bot]

Created attachment 144416 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Alexis Menard (darktears)]

Created attachment 144530 [details]
Patch

[Alexis Menard (darktears)]

Created attachment 144592 [details]
Patch

[Alexis Menard (darktears)]

(In reply to comment #15)
> Created an attachment (id=144592) [details]
> Patch

With -webkit prefixed version as I quote :

"TabAtkins: darktears: Right now, yes, keep using prefixes.  Our feelings about prefix usage and how we should deal with it are still in turmoil, but the current consensus is that we simply need to move fast and monitor usage."

[Eric Seidel (no email)]

Comment on attachment 144592 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:110
> +    CSSPropertyBoxDecorationBreak,

That means we don't add the non-prefixed version, right?

[Alexis Menard (darktears)]

Created attachment 144594 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 144594 [details]
Patch

OK.  I look forward to the rendering patch!

[Alexis Menard (darktears)]

(In reply to comment #19)
> (From update of attachment 144594 [details])
> OK.  I look forward to the rendering patch!

Thanks. Stay around, I will have questions :)

[WebKit Review Bot]

Comment on attachment 144594 [details]
Patch

Clearing flags on attachment: 144594

Committed r118853: <http://trac.webkit.org/changeset/118853>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Paul Irish]

Alexis, can we put this behind a feature flag? 

Clientside feature detection relies on the weak assumption that successful parsing means successful rendering.

[Alexis Menard (darktears)]

(In reply to comment #23)
> Alexis, can we put this behind a feature flag? 
> 
> Clientside feature detection relies on the weak assumption that successful parsing means successful rendering.

Well why not landing https://bugs.webkit.org/show_bug.cgi?id=88228. Eric Seidel is out of the office and David Hyatt is always hard to get.

I can put a feature flags if that's really needed.

[Tab Atkins Jr.]

(In reply to comment #24)
> (In reply to comment #23)
> > Alexis, can we put this behind a feature flag? 
> > 
> > Clientside feature detection relies on the weak assumption that successful parsing means successful rendering.
> 
> Well why not landing https://bugs.webkit.org/show_bug.cgi?id=88228. Eric Seidel is out of the office and David Hyatt is always hard to get.
> 
> I can put a feature flags if that's really needed.

Yes, you should land parsing behind a flag, unless you're quite certain that the working side of it will very shortly follow.  (And if that assumption is broken, it should definitely be behind a flag.)

Given that you appear to be having some problems getting the rendering patch reviewed quickly, I strongly suggest making sure that parsing doesn't work in public until the feature itself works.

[Alexis Menard (darktears)]

(In reply to comment #25)
> (In reply to comment #24)
> > (In reply to comment #23)
> > > Alexis, can we put this behind a feature flag? 
> > > 
> > > Clientside feature detection relies on the weak assumption that successful parsing means successful rendering.
> > 
> > Well why not landing https://bugs.webkit.org/show_bug.cgi?id=88228. Eric Seidel is out of the office and David Hyatt is always hard to get.
> > 
> > I can put a feature flags if that's really needed.
> 
> Yes, you should land parsing behind a flag, unless you're quite certain that the working side of it will very shortly follow.  (And if that assumption is broken, it should definitely be behind a flag.)
> 
> Given that you appear to be having some problems getting the rendering patch reviewed quickly, I strongly suggest making sure that parsing doesn't work in public until the feature itself works.

Ok I will do that then.