Bug 41201

Summary: CSS3 background: Number of layers should be determined by background-image element count
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: bdakin, benjamin, commit-queue, hyatt, koivisto, mitz, simon.fraser, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 42173, 43592    
Bug Blocks:    
Attachments:
Description Flags
Proposed patch
none
Proposed patch v2
none
Proposed patch v3 none

Attachments
Proposed patch (1.89 KB, patch)
2010-06-25 02:27 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Proposed patch v2 (3.95 KB, patch)
2010-07-12 15:56 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Proposed patch v3 (5.82 KB, patch)
2010-07-16 06:09 PDT, Andreas Kling 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Andreas Kling
Comment 1 2010-06-25 02:27:58 PDT
Created attachment 59745 [details] Proposed patch Slightly altered culling logic in FillLayer to discard all layers after (and including) the first one with no image set. Would like some suggestions on how to test this.
WebKit Review Bot
Comment 2 2010-06-25 02:32:00 PDT
Attachment 59745 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/rendering/style/FillLayer.cpp:253: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] WebCore/rendering/style/FillLayer.cpp:254: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Andreas Kling
Comment 3 2010-07-12 15:56:56 PDT
Created attachment 61280 [details] Proposed patch v2 I spoke with Antti about this on IRC, he suggested I make the logic a bit more readable. I've also included a manual test.
WebKit Commit Bot
Comment 4 2010-07-13 04:05:06 PDT
Comment on attachment 61280 [details] Proposed patch v2 Clearing flags on attachment: 61280 Committed r63192: <http://trac.webkit.org/changeset/63192>
WebKit Commit Bot
Comment 5 2010-07-13 04:05:10 PDT
All reviewed patches have been landed. Closing bug.
Antonio Gomes
Comment 6 2010-07-13 12:56:52 PDT
Should this be re-opened since bug 42173 reverted it.
Andreas Kling
Comment 7 2010-07-13 13:28:46 PDT
Yeah, reopening.
Andreas Kling
Comment 8 2010-07-16 05:50:26 PDT
We need to fillUnsetProperties() before we cullEmptyLayers() Consider the following scenario: #foo { background-image: url('Layer1'), ('Layer2'); background-repeat: no-repeat; } The previous version of the patch would discard Layer2 since it had no corresponding properties. If we instead fillUnsetProperties() first, we get the correct result: #foo { background-image: url('Layer1'), ('Layer2'); background-repeat: no-repeat, no-repeat; } Now for the reverse scenario: #foo { background-image: url('Layer1'); background-repeat: no-repeat, no-repeat; } cullEmptyLayers() would previously remove the second 'no-repeat' before fillUnsetProperties() had a chance to repeat Layer1. If we swap the order of fillUnsetProperties() and cullEmptyLayers() we need to alter fillUnsetProperties() a bit - it shouldn't repeat image properties.
Andreas Kling
Comment 9 2010-07-16 06:09:53 PDT
Created attachment 61796 [details] Proposed patch v3
WebKit Commit Bot
Comment 10 2010-07-19 07:06:15 PDT
Comment on attachment 61796 [details] Proposed patch v3 Clearing flags on attachment: 61796 Committed r63653: <http://trac.webkit.org/changeset/63653>
WebKit Commit Bot
Comment 11 2010-07-19 07:06:22 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 12 2010-08-05 15:51:11 PDT
This broke multiple background image: bug 43581.
Simon Fraser (smfr)
Comment 13 2010-08-05 16:59:46 PDT
Rolling this out via bug 43592.
Simon Fraser (smfr)
Comment 14 2010-09-26 08:38:06 PDT
Reopening, since this was rolled out.
Andreas Kling
Comment 15 2011-03-11 12:17:53 PST
*** This bug has been marked as a duplicate of bug 47159 ***
Note You need to log in before you can comment on or make changes to this bug.