Bug 93011 – [CSS Shaders] Add IDL file and bindings for mix function

• Component: CSS
• Status: RESOLVED FIXED

[Max Vujovic]

In this example:

-webkit-filter: custom(none mix(shader.frag normal src-over));

It would be nice if "mix(...)" shows up in CSSOM as WebKitCSSMixFunctionValue instead of CSSValueList.

[Michelangelo De Simone]

Created attachment 174234 [details]
Not for review

[Peter Beverloo (cr-android ews)]

Comment on attachment 174234 [details]
Not for review

Attachment 174234 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14857032

[Build Bot]

Comment on attachment 174234 [details]
Not for review

Attachment 174234 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14846328

New failing tests:
inspector-protocol/nmi-webaudio.html

[Michelangelo De Simone]

Created attachment 174478 [details]
Patch

[Michelangelo De Simone]

Created attachment 174507 [details]
Patch (Rebased)

[Alexandru Chiculita]

Comment on attachment 174507 [details]
Patch (Rebased)

View in context: https://bugs.webkit.org/attachment.cgi?id=174507&action=review

Looks good!

> Source/WebCore/css/WebKitCSSMixFunctionValue.idl:32
> +] interface WebKitCSSMixFunctionValue : CSSValueList {

I think you also need to add the "WebKitCSSMixFunctionValueConstructor" to the DOMWindow.idl . Also, I think there's a test you need to update after that.

attribute CSSValueConstructor CSSValue;
attribute CSSPrimitiveValueConstructor CSSPrimitiveValue;
attribute CSSValueListConstructor CSSValueList;
attribute WebKitCSSTransformValueConstructor WebKitCSSTransformValue;

> LayoutTests/ChangeLog:15
> +        * platform/chromium/css3/filters/custom/custom-filter-mix-bindings-expected.txt: Added.

I think you need to explain why Chromium fails two of the tests.

> LayoutTests/css3/filters/script-tests/custom-filter-mix-bindings.js:39
> +var mixRule = filterRule[0][0][1]

I think mixRule is actually the mixFunction, right?

Should you also test the variables inside the rule?

[Dean Jackson]

Comment on attachment 174507 [details]
Patch (Rebased)

View in context: https://bugs.webkit.org/attachment.cgi?id=174507&action=review

>> Source/WebCore/css/WebKitCSSMixFunctionValue.idl:32
>> +] interface WebKitCSSMixFunctionValue : CSSValueList {
> 
> I think you also need to add the "WebKitCSSMixFunctionValueConstructor" to the DOMWindow.idl . Also, I think there's a test you need to update after that.
> 
> attribute CSSValueConstructor CSSValue;
> attribute CSSPrimitiveValueConstructor CSSPrimitiveValue;
> attribute CSSValueListConstructor CSSValueList;
> attribute WebKitCSSTransformValueConstructor WebKitCSSTransformValue;

Alex is right. You'll need to update two tests. One is window, the other is an SVG version of the same.

[Alexandru Chiculita]

(In reply to comment #7)
> (From update of attachment 174507 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174507&action=review
> 
> >> Source/WebCore/css/WebKitCSSMixFunctionValue.idl:32
> >> +] interface WebKitCSSMixFunctionValue : CSSValueList {
> > 
> > I think you also need to add the "WebKitCSSMixFunctionValueConstructor" to the DOMWindow.idl . Also, I think there's a test you need to update after that.
> > 
> > attribute CSSValueConstructor CSSValue;
> > attribute CSSPrimitiveValueConstructor CSSPrimitiveValue;
> > attribute CSSValueListConstructor CSSValueList;
> > attribute WebKitCSSTransformValueConstructor WebKitCSSTransformValue;
> 
> Alex is right. You'll need to update two tests. One is window, the other is an SVG version of the same.

Michelangelo, can you check if the other CSS Filters related IDL files are added to the window object correctly? If not, could you add bugs for them?

[Michelangelo De Simone]

(In reply to comment #8)

> Michelangelo, can you check if the other CSS Filters related IDL files are added to the window object correctly? If not, could you add bugs for them?

Aye aye, sir!

[Michelangelo De Simone]

Looks like that the supposed infamous test (which I encountered in the past) is not there anymore. I'll rebase the patch with minor modifications and upload it for r? again.

If somebody has more information about this...

[Michelangelo De Simone]

Created attachment 175039 [details]
Patch

[Michelangelo De Simone]

The test we were all talking about is fast/js/global-constructors.html which is actually in the process of being removed (see #90833).
As of now, it's already skipped on Mac and MacWin.

I'll rebaseline anyway and upload a new patch momentarily.

[Michelangelo De Simone]

Created attachment 175058 [details]
Patch

[Dean Jackson]

r+ing this. For years now we've been told to stop using CSSValueList for things, but until someone takes on the task of redesigning the CSS OM we're stuck.

[WebKit Review Bot]

Comment on attachment 175058 [details]
Patch

Rejecting attachment 175058 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ed.txt
patching file LayoutTests/platform/qt-5.0/fast/js/global-constructors-expected.txt
Hunk #1 succeeded at 322 (offset 2 lines).
patching file LayoutTests/platform/qt/fast/js/global-constructors-expected.txt
Hunk #1 succeeded at 305 (offset 2 lines).
patching file LayoutTests/platform/win/fast/js/global-constructors-expected.txt

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Dean Jacks..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14985785

[Michelangelo De Simone]

Created attachment 176035 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 176035 [details]
Patch for landing

Clearing flags on attachment: 176035

Committed r135749: <http://trac.webkit.org/changeset/135749>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.