Test ---- http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/Unbound-background-clip-from-shorthand.html original author is Dirk Schulze Actual results -------------- These 9 background shorthand tests fail: Testing: "background: content-box none repeat scroll 0% 0% / auto padding-box" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: none content-box repeat scroll 0% 0% / auto padding-box" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: none repeat scroll content-box 0% 0% / auto padding-box" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: content-box none padding-box repeat scroll 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: content-box none repeat padding-box scroll 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: content-box none repeat scroll padding-box 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: none content-box repeat padding-box scroll 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: none content-box repeat scroll padding-box 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Testing: "background: none repeat content-box scroll padding-box 0% 0% / auto" Failed expected background-origin: 'content-box'; actual background-origin: 'padding-box'. Failed expected background-clip: 'padding-box'; actual background-clip: 'border-box'. Explanation ----------- <bg-layer> = <bg-image> || <position> [ / <bg-size> ]? || <repeat-style> || <attachment> || <box> || <box> CSS3 Backgrounds and Borders, section 3.10. Backgrounds Shorthand: the ‘background’ property http://www.w3.org/TR/css3-background/#the-background " A double bar (||) separates two or more options: one or more of them must occur, in any order. " http://www.w3.org/TR/CSS21/about.html#value-defs " If one <box> value is present then it sets both ‘background-origin’ and ‘background-clip’ to that value. If two values are present, then the first sets ‘background-origin’ and the second ‘background-clip’. " 3.10. Backgrounds Shorthand: the ‘background’ property http://www.w3.org/TR/css3-background/#the-background The provided test is testing situation when/where there are 2 <box> values and when their respective order is different in the background shorthand. More info --------- - The provided test could be split into 9 separate, distinct tests with an expected rendered layout - Firefox 39 and Firefox 42.0a1 buildID=20150718201542 fail this test - IE11, Chrome 44.0.2403.107 and Opera 12.16 pass this test - Re: [css3-background] unbound background-clip from -origin in background shorthand http://lists.w3.org/Archives/Public/www-style/2012Nov/0542.html - I've searched for duplicates and could not find one... although bug 570896 must be related

This is needed to match the spec change in https://hg.csswg.org/drafts/rev/7e4cec622a1c9d999a90c3ec4f091c7ebd10ff1e . See also https://github.com/servo/servo/issues/15772 .

Gecko needs to fix this to have same behavior as stylo.

I have a feeling that how we fix this may affect other things, specifically, how "text" is allowed in background shorthand. Currently, Gecko allows "text" to present in background shorthand if it immediately follows another <box> value. The spec isn't clear on this, because background-clip: text is added in css-backgrounds-4 as an independent value of background-clip (not a value of <box>), while in css-backgrounds-3, the <bg-layer> just use "<box> || <box>" for this. I don't think it makes sense to add "text" to <box>, so if the spec wants to add "text" to background shorthand, it would need to do something like "<box> || [ <box> | text ]", which also means the order of clip and origin can be reversed if one use text for clip. I don't know whether that would be a problem. FWIW, Blink doesn't support "text" in background shorthand, and Edge supports it but requires the order of origin and clip even if clip is "text". I suggest we remove the support of "text" in background shorthand when we fix this bug, and we can always add it back when the spec becomes clear.

Ok, so I'm now looking at the code, and it seems css-masking does use "<geometry-box> || [ <geometry-box> | no-clip ]" so I suppose css-backgrounds is going to do the same thing. Then it is probably easier to just support "text" and allow it to come before background-origin.

Comment on attachment 8871144 [details] Bug 1188074 part 1 - Refactor ParseImageLayersItem a bit to check whether a slot is filled before checking whether the keyword matches. https://reviewboard.mozilla.org/r/142660/#review146334 Yeah, seems better for it to be a bit slower when the value has a syntax error due to repeated values, than to keep looking up the keywords for already filled slots.

Comment on attachment 8871145 [details] Bug 1188074 part 2 - Allow origin and clip values to appear anywhere in an image layer shorthand property. https://reviewboard.mozilla.org/r/142662/#review146338 ::: commit-message-b31d4:1 (Diff revision 2) > +Bug 1188074 part 2 - Implement the combine any syntax for image layer values. r?heycam I'm having trouble parsing this. Maybe "Allow origin and clip values to appear anywhere in an image layer shorthand property."? ::: layout/style/nsCSSParser.cpp:12399 (Diff revision 2) > #ifdef DEBUG > - const KTableEntry* clipTable = nsCSSProps::kKeywordTableTable[aTable[nsStyleImageLayers::clip]]; > - for (size_t i = 0; originTable[i].mValue != -1; i++) { > + for (size_t i = 0; originTable[i].mValue != -1; i++) { > - // For each keyword & value in kOriginKTable, ensure that > + // For each keyword & value in kOriginKTable, ensure that > - // kBackgroundKTable has a matching entry at the same position. > + // kBackgroundKTable has a matching entry at the same position. > - MOZ_ASSERT(originTable[i].mKeyword == clipTable[i].mKeyword); > + MOZ_ASSERT(originTable[i].mKeyword == clipTable[i].mKeyword); > - MOZ_ASSERT(originTable[i].mValue == clipTable[i].mValue); > + MOZ_ASSERT(originTable[i].mValue == clipTable[i].mValue); > - } > + } > #endif Maybe we should move these assertions to the top of the function, just after we declare originTable and clipTable, so that it's not so distracting here. And just comment that we've asserted above the values are compatible.

Comment on attachment 8871145 [details] Bug 1188074 part 2 - Allow origin and clip values to appear anywhere in an image layer shorthand property. https://reviewboard.mozilla.org/r/142662/#review146338 > I'm having trouble parsing this. Maybe "Allow origin and clip values to appear anywhere in an image layer shorthand property."? I actually had trouble parsing this myself... but I didn't figure out a much better commit message so I just left it as is. Sorry about that. > Maybe we should move these assertions to the top of the function, just after we declare originTable and clipTable, so that it's not so distracting here. And just comment that we've asserted above the values are compatible. I tried moving it to the top of the function, but later I moved it back because I think this should be put near where we actually want the assertion, so that it is clear that the assertion is related to this code, and when someone wants to refactor it, they know it should or should not be removed as well.

Servo side: servo/servo#17035 They don't really need coordinated landing... that would just cause some increasing of mochitest failures...

Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/934aae5e56b2 part 1 - Refactor ParseImageLayersItem a bit to check whether a slot is filled before checking whether the keyword matches. r=heycam https://hg.mozilla.org/integration/autoland/rev/abcf45c9ad66 part 2 - Allow origin and clip values to appear anywhere in an image layer shorthand property. r=heycam

https://hg.mozilla.org/mozilla-central/rev/934aae5e56b2 https://hg.mozilla.org/mozilla-central/rev/abcf45c9ad66