Source/WebCore/rendering/FilterEffectRenderer.cpp does not compile when the following command is issued: Tools/Scripts/build-webkit --no-svg --no-svg-fonts --no-svg-dom-objc-bindings
Created attachment 154680 [details] surpress unused parameters warning.
Comment on attachment 154680 [details] surpress unused parameters warning. r=me.
Comment on attachment 154680 [details] surpress unused parameters warning. Clearing flags on attachment: 154680 Committed r123781: <http://trac.webkit.org/changeset/123781>
All reviewed patches have been landed. Closing bug.
build is broken again when svg is disabled.
Created attachment 184759 [details] fix patch
Comment on attachment 184759 [details] fix patch Rejecting attachment 184759 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'validate-changelog', '--non-interactive', 184759, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/16121415
Created attachment 184763 [details] fix patch
Comment on attachment 184763 [details] fix patch View in context: https://bugs.webkit.org/attachment.cgi?id=184763&action=review > Source/WebCore/css/StyleResolver.cpp:4110 > -#if ENABLE(SVG) > default: > +#if ENABLE(SVG) This part should not be needed. We're missing some non-svg properties from this list which are being masked by the presence of applySVGProperty. :(
I'm not saying it's your fault. But the build break for non-svg is just being masked in teh SVG case, and is a real bug.
I would be nice to get rid of the no-SVG build entirely. I guess everyone ships it.
before SVG came along, all CSS properties were handled in applyProperty(). For various reasons, SVG property handling was made separate in svgApplyProperty, but they share in enum, thus we had to add that default: clause. Now, since SVG has been enabled for so long, someone must have added a new proprty and failed to support it in applyProperty like they should. We didn't catch it, because both applyProperty and applySVGPrperty use default: clauses since neither of them can handle the whole list of properties in their switch. :(
Comment on attachment 184763 [details] fix patch Clearing flags on attachment: 184763 Committed r140845: <http://trac.webkit.org/changeset/140845>
(In reply to comment #12) > before SVG came along, all CSS properties were handled in applyProperty(). For various reasons, SVG property handling was made separate in svgApplyProperty, but they share in enum, thus we had to add that default: clause. Now, since SVG has been enabled for so long, someone must have added a new proprty and failed to support it in applyProperty like they should. We didn't catch it, because both applyProperty and applySVGPrperty use default: clauses since neither of them can handle the whole list of properties in their switch. :( I agree with you guys. The correct way for the switch should be: case <a non-svg case>: blah-blah; #if ENABLE(SVG) case <a svg case>: blah-blah; #endif default: assert(0); But my fix at least didn't introduce new bugs. :) I just wanted to commit it so I don't have to patch it every time. Thanks.