| Summary: | [CSS3 Backgrounds and Borders] Implement box-decoration-break parsing. | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexis Menard (darktears) <menard> | ||||||||||||||||||||
| Component: | New Bugs | Assignee: | Alexis Menard (darktears) <menard> | ||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||
| Severity: | Normal | CC: | cmarcelo, dglazkov, eric, hyatt, jackalmage, macpherson, paulirish, tabatkins, webkit.review.bot | ||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Alexis Menard (darktears)
2012-05-28 13:38:21 PDT
Created attachment 144394 [details]
Patch
(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. This technically looks fine. Are we supposed to be adding -webkit- these days or not? (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. Comment on attachment 144394 [details] Patch Attachment 144394 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12853048 Created attachment 144396 [details]
Patch
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 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
Created attachment 144406 [details]
Patch
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 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
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 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
Created attachment 144530 [details]
Patch
Created attachment 144592 [details]
Patch
(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." 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? Created attachment 144594 [details]
Patch
Comment on attachment 144594 [details]
Patch
OK. I look forward to the rendering patch!
(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 :) Comment on attachment 144594 [details] Patch Clearing flags on attachment: 144594 Committed r118853: <http://trac.webkit.org/changeset/118853> All reviewed patches have been landed. Closing bug. Alexis, can we put this behind a feature flag? Clientside feature detection relies on the weak assumption that successful parsing means successful rendering. (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. (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. (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. |