Bug 43503 – Implement MathML deprecated style attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: MathML

[François Sausset]

Following attributes are deprecated but still needed by MathML 3: color, background, fontfamily, fontsize, fontstyle, fontweight.

[François Sausset]

Created attachment 63483 [details]
Patch

[WebKit Commit Bot]

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

[François Sausset]

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.

[Eric Seidel (no email)]

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

[François Sausset]

Created attachment 63812 [details]
Revised patch

Removed unnecessary "MathMLNames" and regenerated PNG file.

For the code simplification, I don't really understand the suggestion.

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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

[François Sausset]

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.

[Eric Seidel (no email)]

Comment on attachment 63812 [details]
Revised patch

Clearing flags on attachment: 63812

Committed r64982: <http://trac.webkit.org/changeset/64982>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.