Bug 43503

Created attachment 63483 [details] Patch

Comment on attachment 63483 [details] Patch Rejecting patch 63483 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1 Last 500 characters of output: file LayoutTests/platform/mac/mathml/presentation/attributes-expected.png Hunk #1 FAILED at 1. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/mac/mathml/presentation/attributes-expected.png.rej patching file LayoutTests/platform/mac/mathml/presentation/attributes-expected.txt Hunk #1 FAILED at 1. Hunk #3 succeeded at 74 with fuzz 2 (offset 17 lines). 1 out of 3 hunks FAILED -- saving rejects to file LayoutTests/platform/mac/mathml/presentation/attributes-expected.txt.rej Full output: http://queues.webkit.org/results/3634403

Created attachment 63641 [details] Regenerated patch Layout tests have been regenerated as another patch, which has been landed between the creation and the commit of this patch, modified these tests.

Comment on attachment 63641 [details] Regenerated patch WebCore/mathml/MathMLElement.cpp:54 + || attrName == MathMLNames::colorAttr || attrName == MathMLNames::backgroundAttr Seems this file really wants "using namespace MathMLNames;" WebCore/mathml/MathMLElement.cpp:71 + } else if (attr->name() == MathMLNames::mathcolorAttr) These all do the same thing, no? I might have considered writing a helper function myself. handleCSSMappedAttribute(attr, fontsizeAttr, CSSPropertyFontSize); Then you just have a long line of those, instead of needing an if cascade. :) It's a nit, and not a big deal. Use your discretion. Seems your png files aren't correctly marked as image/png

Created attachment 63812 [details] Revised patch Removed unnecessary "MathMLNames" and regenerated PNG file. For the code simplification, I don't really understand the suggestion.

Comment on attachment 63483 [details] Patch Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 63483 [details] so that this bug does not appear in http://webkit.org/pending-commit.

Comment on attachment 63812 [details] Revised patch Rejecting patch 63812 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1 Last 500 characters of output: ttp/tests/xmlhttprequest/access-control-basic-non-simple-allow-async.html -> timed out Sampling process 22949 for 10 seconds with 10 milliseconds of run time between samples Sampling completed, processing symbols... Sample analysis of process 22949 written to file /Users/eseidel/Library/Logs/DumpRenderTree/HangReport.txt Exiting early after 1 failures. 20760 tests run. 523.37s total testing time 20759 test cases (99%) succeeded 1 test case (<1%) timed out 89 test cases (<1%) had stderr output Full output: http://queues.webkit.org/results/3687049

Comment on attachment 63812 [details] Revised patch I switched commit-queue to ? again, as I think that failures are not related to this patch. I tested on my computer r64967 with and without this patch and there were the same failures in the two cases.

Comment on attachment 63812 [details] Revised patch Clearing flags on attachment: 63812 Committed r64982: <http://trac.webkit.org/changeset/64982>

All reviewed patches have been landed. Closing bug.