Bug 261552 – getComputedStyle(elt).backgroundBlendMode returns only the first property of a list (background-blend-mode)

• Component: CSS
• Status: NEW

[Karl Dubost]

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

[Radar WebKit Bug Importer]

<rdar://problem/115832043>

[Karl Dubost]

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));
    }

[Karl Dubost]

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"
```

[Karl Dubost]

That's not it. 

* Not because of layers
* Not because of Comma vs Space

[Karl Dubost]

Maybe layers, only 1?

[Simon Fraser (smfr)]

I think it is because it should be createCommaSeparated(). See the similar code for case CSSPropertyBackgroundImage:

[Karl Dubost]

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.

[Karl Dubost]

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())

[Simon Fraser (smfr)]

This depends on whether the background-image property has > 1 image, since, in webkit, that determines the length of the FillLayers list.

[Karl Dubost]

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.

[Karl Dubost]

Note tests will be fixed/extended in 
https://github.com/web-platform-tests/wpt/issues/42496

[Karl Dubost]

I opened a bug on Mozilla bugzilla too. 
https://bugzilla.mozilla.org/show_bug.cgi?id=1858591

[Karl Dubost]

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).

[Karl Dubost]

Created attachment 468185 [details]
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

[Karl Dubost]

Created Bug 263104