Bug 41201 – CSS3 background: Number of layers should be determined by background-image element count

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: CSS

[Andreas Kling]

This test on IE Test Center fails:
http://samples.msdn.microsoft.com/ietestcenter/css3/bordersbackgrounds/background_properties_greater_than_images.htm

Spec link:
http://www.w3.org/TR/css3-background/#layering

[Andreas Kling]

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]

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]

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 on attachment 61280 [details]
Proposed patch v2

Clearing flags on attachment: 61280

Committed r63192: <http://trac.webkit.org/changeset/63192>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Antonio Gomes]

Should this be re-opened since bug 42173 reverted it.

[Andreas Kling]

Yeah, reopening.

[Andreas Kling]

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]

Created attachment 61796 [details]
Proposed patch v3

[WebKit Commit Bot]

Comment on attachment 61796 [details]
Proposed patch v3

Clearing flags on attachment: 61796

Committed r63653: <http://trac.webkit.org/changeset/63653>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This broke multiple background image: bug 43581.

[Simon Fraser (smfr)]

Rolling this out via bug 43592.

[Simon Fraser (smfr)]

Reopening, since this was rolled out.

[Andreas Kling]

*** This bug has been marked as a duplicate of bug 47159 ***