261552 2023-09-14 02:28:52 -0700 getComputedStyle(elt).backgroundBlendMode returns only the first property of a list (background-blend-mode) 2023-10-12 19:45:41 -0700 1 1 1 Unclassified WebKit CSS Safari 17 Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=263051 http://wpt.live/css/compositing/parsing/background-blend-mode-computed.html BrowserCompat, InRadar, WPTImpact P2 Normal --- 263104 1 karlcow webkit-unassigned emilio heycam koivisto sabouhallawa simon.fraser webkit-bug-importer oldest_to_newest 1977465 0 karlcow 2023-09-14 02:28:52 -0700 see http://wpt.live/css/compositing/parsing/background-blend-mode-computed.html when "normal, luminosity" is specified it returns only "normal" while the "normal, luminosity" values are considered supported. Firefox passes the test. http://wpt.fyi/css/compositing/parsing/background-blend-mode-computed.html 1978930 1 webkit-bug-importer 2023-09-21 02:29:13 -0700 <rdar://problem/115832043> 1984236 2 karlcow 2023-10-11 00:38:31 -0700 https://searchfox.org/wubkat/rev/e21786e1635702a6ca7c1e30290f32e8a97fec23/Source/WebCore/css/ComputedStyleExtractor.cpp#4101-4109 case CSSPropertyBackgroundBlendMode: { auto& layers = style.backgroundLayers(); if (!layers.next()) return createConvertingToCSSValueID(layers.blendMode()); CSSValueListBuilder list; for (auto* currLayer = &layers; currLayer; currLayer = currLayer->next()) list.append(createConvertingToCSSValueID(currLayer->blendMode())); return CSSValueList::createSpaceSeparated(WTFMove(list)); } 1984238 3 karlcow 2023-10-11 00:55:50 -0700 I do not have much knowledge on this code, but the `getComputedStyle()` seems to take into account a list of values `CSSValueList::createSpaceSeparated(WTFMove(list));`. So maybe this is related to the computation of `auto& layers = style.backgroundLayers();` Probably Antti is a better person for understanding what is the source of the issue. Once understood Maybe it will be easy to fix. oh wait, is it because it should be comma instead of space. `CSSValueList::createCommaSeparated(WTFMove(list));`. instead of: `CSSValueList::createSpaceSeparated(WTFMove(list));`. ``` > target.style.backgroundBlendMode < "screen, overlay" > getComputedStyle(target)['background-blend-mode'] < "screen" ``` 1984241 4 karlcow 2023-10-11 01:18:05 -0700 That's not it. * Not because of layers * Not because of Comma vs Space 1984242 5 karlcow 2023-10-11 01:33:53 -0700 Maybe layers, only 1? 1984392 6 simon.fraser 2023-10-11 14:10:06 -0700 I think it is because it should be createCommaSeparated(). See the similar code for case CSSPropertyBackgroundImage: 1984411 7 karlcow 2023-10-11 14:54:45 -0700 yes, I quickly tried to replace by createCommaSeparated, but it didn't fix it. So I added different WTFLogAlways() at a couple of places. It never goes in that part part of the code with CSSValueList::createCommaSeparated(WTFMove(list)) So there must be something else too. I will try again to make double sure. 1984466 8 karlcow 2023-10-11 17:04:58 -0700 Simon, I tried again by just changing to make sure. And it's not only the createCommaSeparated/createSpaceSeparated diff --git a/Source/WebCore/css/ComputedStyleExtractor.cpp b/Source/WebCore/css/ComputedStyleExtractor.cpp index b3c66f42e0f0..0b652382f480 100644 --- a/Source/WebCore/css/ComputedStyleExtractor.cpp +++ b/Source/WebCore/css/ComputedStyleExtractor.cpp @@ -4099,13 +4099,19 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty return createConvertingToCSSValueID(style.isolation()); #endif case CSSPropertyBackgroundBlendMode: { + WTFLogAlways("Inside CSSPropertyBackgroundBlendMode"); auto& layers = style.backgroundLayers(); - if (!layers.next()) + if (!layers.next()) { + WTFLogAlways("Early return createConvertingToCSSValueID(layers.blendMode())"); return createConvertingToCSSValueID(layers.blendMode()); + } CSSValueListBuilder list; for (auto* currLayer = &layers; currLayer; currLayer = currLayer->next()) list.append(createConvertingToCSSValueID(currLayer->blendMode())); - return CSSValueList::createSpaceSeparated(WTFMove(list)); + // we want a comma separated list, + // but are we reaching this line of code? + WTFLogAlways("final return CSSValueList::createCommaSeparated(WTFMove(list))"); + return CSSValueList::createCommaSeparated(WTFMove(list)); } case CSSPropertyBackground: return getBackgroundShorthandValue(); and loading: http://wpt.live/css/compositing/parsing/background-blend-mode-computed.html it never goes to the last return for the list of values. % run-minibrowser debug Starting MiniBrowser with DYLD_FRAMEWORK_PATH set to point to built WebKit in /Users/karlcow/code/OpenSource/WebKitBuild/Debug. Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) Inside CSSPropertyBackgroundBlendMode Early return createConvertingToCSSValueID(layers.blendMode()) 1984473 9 simon.fraser 2023-10-11 17:21:09 -0700 This depends on whether the background-image property has > 1 image, since, in webkit, that determines the length of the FillLayers list. 1984493 10 karlcow 2023-10-11 18:14:01 -0700 So in Bug 263051, the createCommaSeparated() list is being fixed. But that doesn't solve this issue as explained in the comments above. There could be a test bug, but it's not totally sure either. http://wpt.live/css/compositing/parsing/background-blend-mode-computed.html Simon is saying: > there should be a FillLayer per background value; the length of the list is determined by background-image/mask-image but we should double check the spec first to see how this is supposed to work. https://drafts.fxtf.org/compositing-1/#background-blend-mode > Defines the blending mode of each background layer. > > Each background layer must blend with the element’s background layer > that is below it and the element’s background color. Background layers > must not blend with the content that is behind the element, instead > they must act as if they are rendered into an isolated group. and > The background-blend-mode list must be applied in the same order > as background-image [CSS3BG]. This means that the first element > in the list will apply to the layer that is on top. If a property > doesn’t have enough comma-separated values to match the number > of layers, the UA must calculate its used value by repeating the > list of values until there are enough. The layering seems to be defined in https://drafts.csswg.org/css-backgrounds-3/#layering It says: > Each of the images is sized, positioned, and tiled according > to the corresponding value in the other background properties. Then > The lists are matched up from the first value: excess values > at the end are not used. Then > If a property doesn’t have enough comma-separated values > to match the number of layers, the UA must calculate its used value > by repeating the list of values until there are enough. I need to read this carefully again. The example in this section background-image: url(flower.png), url(ball.png), url(grass.png); background-position: center center, 20% 80%, top left, bottom right; background-origin: border-box, content-box; background-repeat: no-repeat; has exactly the same effect as this set with the extra position dropped and the missing values for background-origin and background-repeat filled in (emphasized for clarity): background-image: url(flower.png), url(ball.png), url(grass.png); background-position: center center, 20% 80%, top left; background-origin: border-box, content-box, border-box; background-repeat: no-repeat, no-repeat, no-repeat; And we can see that for background-position where there is 4 values list with an extra "bottom right", it drops it to make it a 3 values list. So from this I would say the test as currently defined on WPT is wrong and probably the implementation of Gecko too. 1984507 11 karlcow 2023-10-11 19:41:57 -0700 Note tests will be fixed/extended in https://github.com/web-platform-tests/wpt/issues/42496 1984531 12 karlcow 2023-10-11 22:47:36 -0700 I opened a bug on Mozilla bugzilla too. https://bugzilla.mozilla.org/show_bug.cgi?id=1858591 1984551 13 karlcow 2023-10-12 00:35:39 -0700 I was too hasty here. See the discussions on the tests fix! https://github.com/web-platform-tests/wpt/issues/42496#issuecomment-1759017241 The latest discussions in the CSS WG https://github.com/w3c/csswg-drafts/issues/7164#issuecomment-1201340918 led fantasai to clarify the way this is working https://drafts.csswg.org/css-values-4/#linked-properties Specifically this sentence: > The computed values of the coordinating list properties are not affected by such truncation or repetition. which means getComputedStyle() should return the same list than what was specified. (even if the used values have been truncated or repeated.) So there is something which needs to be fixed in WebKit (and Blink). 1984560 14 468185 karlcow 2023-10-12 01:09:31 -0700 Created attachment 468185 testcase it has a bigger impact than just backgroundBlendMode. We might want to rename this bug. getComputedStyle(elt).* should return the specified values for coordinating list properties. .test { background-image: url(foo.ping); background-blend-mode: normal, luminosity; background-position: 0px 0px, 10px 10px; } let test = document.querySelector('.test'); window.getComputedStyle(test).backgroundBlendMode; window.getComputedStyle(test).backgroundPosition; should return (like Gecko) normal, luminosity 0px 0px, 10px 10px currently WebKit and Blink return normal 0px 0px 1984800 15 karlcow 2023-10-12 19:45:41 -0700 Created Bug 263104 468185 2023-10-12 01:09:31 -0700 2023-10-12 01:09:31 -0700 testcase computed-background-linked-list-values.html text/html 1136 karlcow PCFET0NUWVBFIGh0bWw+CjxodG1sPgo8aGVhZD4KICAgIDxtZXRhIG5hbWU9InZpZXdwb3J0IiBj b250ZW50PSJ3aWR0aD1kZXZpY2Utd2lkdGgsIGluaXRpYWwtc2NhbGU9MSwgc2hyaW5rLXRvLWZp dD1ubyI+CiAgICA8dGl0bGU+PC90aXRsZT4KICAgIDxzdHlsZT4KICAgICAgICAuYm94IHsKICAg ICAgICAgICAgd2lkdGg6IDEwMHB4OwogICAgICAgICAgICBoZWlnaHQ6IDEwMHB4OwogICAgICAg ICAgICBiYWNrZ3JvdW5kLWNvbG9yOiBibHVlOwogICAgICAgICAgICBiYWNrZ3JvdW5kLWltYWdl OiB1cmwoZm9vLnBpbmcpOwogICAgICAgICAgIC8qIGJhY2tncm91bmQtaW1hZ2U6IHVybChmb28u cGluZyksIHVybChiYXIucG5nKTsgKi8KICAgICAgICAgICAgYmFja2dyb3VuZC1ibGVuZC1tb2Rl OiBub3JtYWwsIGx1bWlub3NpdHk7CiAgICAgICAgICAgIGJhY2tncm91bmQtcG9zaXRpb246IDBw eCAwcHgsIDEwcHggMTBweDsKICAgICAgICB9CiAgICAgICAgCiAgICA8L3N0eWxlPgogICAgPHNj cmlwdD4KICAgICAgICB3aW5kb3cuYWRkRXZlbnRMaXN0ZW5lcignbG9hZCcsICgpID0+IHsKICAg ICAgICAgICAgbGV0IGJveCA9IGRvY3VtZW50LnF1ZXJ5U2VsZWN0b3IoJy5ib3gnKTsKICAgICAg ICAgICAgbGV0IGNvbXB1dGVkU3R5bGUxID0gd2luZG93LmdldENvbXB1dGVkU3R5bGUoYm94KS5i YWNrZ3JvdW5kQmxlbmRNb2RlOwogICAgICAgICAgICBsZXQgY29tcHV0ZWRTdHlsZTIgPSB3aW5k b3cuZ2V0Q29tcHV0ZWRTdHlsZShib3gpLmJhY2tncm91bmRQb3NpdGlvbjsKICAgICAgICAgICAg ZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoJ3Jlc3VsdCcpLnRleHRDb250ZW50ID0gJ2dldENTKCku YmFja2dyb3VuZEJsZW5kTW9kZTogJyArIGNvbXB1dGVkU3R5bGUxICsgJ1xuJwogICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgKyAnZ2V0Q1Mo KS5iYWNrZ3JvdW5kUG9zaXRpb246ICcgKyBjb21wdXRlZFN0eWxlMjsKICAgICAgICB9LCBmYWxz ZSk7CiAgICA8L3NjcmlwdD4KPC9oZWFkPgo8Ym9keT4KICAgIDxkaXYgY2xhc3M9ImJveCI+CiAg ICA8L2Rpdj4KICAgIDxwcmUgaWQ9InJlc3VsdCI+PC9wcmU+CjwvYm9keT4KPC9odG1sPgo=