Bug 43503 - Implement MathML deprecated style attributes
: Implement MathML deprecated style attributes
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: MathML
: 528+ (Nightly build)
: Macintosh Intel Mac OS X 10.6
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 3251
  Show dependency treegraph
 
Reported: 2010-08-04 12:42 PDT by François Sausset
Modified: 2010-08-09 08:40 PDT (History)
2 users (show)

See Also:


Attachments
Patch (57.21 KB, patch)
2010-08-04 12:53 PDT, François Sausset
no flags Details | Formatted Diff | Diff
Regenerated patch (61.75 KB, patch)
2010-08-05 15:03 PDT, François Sausset
no flags Details | Formatted Diff | Diff
Revised patch (61.70 KB, patch)
2010-08-07 02:48 PDT, François Sausset
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description François Sausset 2010-08-04 12:42:27 PDT
Following attributes are deprecated but still needed by MathML 3: color, background, fontfamily, fontsize, fontstyle, fontweight.
Comment 1 François Sausset 2010-08-04 12:53:47 PDT
Created attachment 63483 [details]
Patch
Comment 2 WebKit Commit Bot 2010-08-05 13:38:11 PDT
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
Comment 3 François Sausset 2010-08-05 15:03:56 PDT
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 4 Eric Seidel 2010-08-06 14:36:54 PDT
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
Comment 5 François Sausset 2010-08-07 02:48:33 PDT
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 6 Eric Seidel 2010-08-08 03:17:04 PDT
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 7 Eric Seidel 2010-08-09 00:41:39 PDT
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 8 François Sausset 2010-08-09 03:03:26 PDT
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 9 Eric Seidel 2010-08-09 08:40:49 PDT
Comment on attachment 63812 [details]
Revised patch

Clearing flags on attachment: 63812

Committed r64982: <http://trac.webkit.org/changeset/64982>
Comment 10 Eric Seidel 2010-08-09 08:40:55 PDT
All reviewed patches have been landed.  Closing bug.