Bug 93011

Created attachment 174234 [details] Not for review

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

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

Created attachment 174478 [details] Patch

Created attachment 174507 [details] Patch (Rebased)

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?

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.

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

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

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

Created attachment 175039 [details] Patch

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.

Created attachment 175058 [details] Patch

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.

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

Created attachment 176035 [details] Patch for landing

Comment on attachment 176035 [details] Patch for landing Clearing flags on attachment: 176035 Committed r135749: <http://trac.webkit.org/changeset/135749>

All reviewed patches have been landed. Closing bug.