As indicated in http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree and as I explained in bug 118053 comment 7, using pixel tests is too fragile especially for MathML where the rendering is not defined precisely (as opposed to e.g. SVG or CSS). Basically, if we start to change anything like fixing bug 115787 or the patch Martin Robinson is working on for stretchy operators, that will just break all the MathML tests. Hence one has to verify the tests by eye and regenerate the references again which is error-prone and a bit in contradiction with the goal of automated regression tests. Moreover it is not always clear what's the tests are verifying (position of scripts? sizes? spacing? font style? everything at the same time?) and currently references seem to only available on GTK, EFL and Mac (so I guess the other platforms just load the pages without verifying against a references?) I suggest to replace them by reftests. We won't be able to test the subjective fact that a formula is "good-looking" and the test pages may be somewhat less "natural". But at least they become easy to verify (the two pages should just match / not match), they allow to restrict the unit test to only the minimal feature you want to check and ignore unrelated stuff, they won't break so easily and will be more cross-compatible (in particular no need to generate the references for each platform).
Created attachment 206530 [details] Patch V1 A first version that converts some of the tests. For scripts: I assume all what we want to test is that the scripts are centered above/below the base and that they are drawn smaller (although I don't think the scriptlevel stuff is implemented correctly in WebKit). There seems to be a test for largeop in inline mode (displaystyle=false) for which under/overscripts should be placed as sub/supscripts but that does not seem to work in WebKit. I've created a separate test for bug 95404. For tokens: I kept the testing of regular/italic. WebKit does not implement any MathML spacing yet nor does it draw quotes around ms, so it's not worth trying to do something more complicated.
Created attachment 206532 [details] Patch V1 - bis
Comment on attachment 206532 [details] Patch V1 - bis Attachment 206532 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1064249 New failing tests: mathml/presentation/attributes3.html
Created attachment 206588 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Comment on attachment 206532 [details] Patch V1 - bis Attachment 206532 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1045564 New failing tests: mathml/presentation/attributes3.html
Created attachment 206604 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Created attachment 206749 [details] Patch V2
Created attachment 206752 [details] Patch V2 - bis
Comment on attachment 206752 [details] Patch V2 - bis Attachment 206752 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1089173 New failing tests: mathml/presentation/bug97990.html mathml/presentation/scripts4.html
Created attachment 206763 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Created attachment 206771 [details] Patch V3
Comment on attachment 206771 [details] Patch V3 OK, the new MathML tests pass now. I haven't converted everything yet but this patch is already big so I'll finish that in a follow-up bug.
Comment on attachment 206771 [details] Patch V3 View in context: https://bugs.webkit.org/attachment.cgi?id=206771&action=review > LayoutTests/mathml/presentation/attributes1-expected-mismatch.html:12 > + <mn>1</mn> bad indendation > LayoutTests/mathml/presentation/attributes1.html:1 > +<!doctype html> can we make the names more descriptive, so if something fails the title will give away what should be looked at so this would be attributes-style. > LayoutTests/mathml/presentation/attributes2.html:1 > +<!doctype html> attributes-background-color.html > LayoutTests/mathml/presentation/attributes2.html:14 > + <math> bad indentation of <math> in comparison to children tags > LayoutTests/mathml/presentation/attributes2.html:29 > + <math mathbackground="orange"> ditto > LayoutTests/mathml/presentation/attributes3.html:1 > +<!doctype html> attributes-mathsize.html > LayoutTests/mathml/presentation/attributes4-expected.html:14 > + "U+1D468 MATHEMATICAL BOLD ITALIC CAPITAL A") but WebKit implementation but "the" WebKit imple... > LayoutTests/mathml/presentation/attributes4.html:2 > +<html> attributes-mathvariant.html > LayoutTests/mathml/presentation/attributes4.html:10 > + single-char mi elements and "normal" otherwise. --> single-char <mi> elements > LayoutTests/mathml/presentation/attributes5-expected.html:9 > + <!-- This only draws the background boxes since the x is hidden. --> what X are you referring to here? are you referring to the content inside the tag? > LayoutTests/mathml/presentation/attributes5.html:3 > + <head> attributes-display.html > LayoutTests/mathml/presentation/attributes5.html:27 > + background: blue;"> ditto for indendation > LayoutTests/mathml/presentation/bug95015-expected.html:10 > + <!-- The msubsup should be in supscript and so hidden by the red should be in "a" subscript, so "it should be" hidden by.. > LayoutTests/mathml/presentation/bug95015.html:10 > + <!-- The msubsup should be in supscript and so hidden by the red ditto for comment > LayoutTests/mathml/presentation/bug95404-expected.html:16 > + background: red; width: 100%;"> are you wrapping to 80 chars for every line? it seems like a lot of these lines would be easier to read on one line > LayoutTests/mathml/presentation/scripts1.html:2 > +<html> this probably deserves two tests scripts-underover scripts-supsub > LayoutTests/mathml/presentation/scripts1.html:208 > + not currently implement scriptlevel and only sets the size to bug number? > LayoutTests/mathml/presentation/scripts2.html:2 > +<html> scripts-fontsize
(In reply to comment #1) > Created an attachment (id=206530) [details] > Patch V1 > > A first version that converts some of the tests. > > For scripts: I assume all what we want to test is that the scripts are centered above/below the base and that they are drawn smaller (although I don't think the scriptlevel stuff is implemented correctly in WebKit). There seems to be a test for largeop in inline mode (displaystyle=false) for which under/overscripts should be placed as sub/supscripts but that does not seem to work in WebKit. I've created a separate test for bug 95404. > (In reply to comment #13) > > LayoutTests/mathml/presentation/scripts1.html:208 > > + not currently implement scriptlevel and only sets the size to > > bug number? I've opened bug 118737 and bug 118738 for these issues.
Created attachment 206806 [details] Patch V4
Comment on attachment 206806 [details] Patch V4 Thanks
Comment on attachment 206806 [details] Patch V4 Rejecting attachment 206806 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 206806, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/1080334
Created attachment 206821 [details] Patch Final Version Oops, once again I've badly updated the ChangeLog after having addressed the review comments (I called prepare-Changelog and updated the file by hand, but I'm wondering if there is a better way to update the list of modified files)
Comment on attachment 206821 [details] Patch Final Version Attachment 206821 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1110039 New failing tests: fullscreen/full-screen-iframe-with-max-width-height.html
Created attachment 206840 [details] Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Comment on attachment 206821 [details] Patch Final Version Clearing flags on attachment: 206821 Committed r152777: <http://trac.webkit.org/changeset/152777>
All reviewed patches have been landed. Closing bug.
Note: this adds expected-mismatch reftests that are likely to fail on Qt bots and should be skipped: + * mathml/presentation/attributes-style-expected-mismatch.html: Added. + * mathml/presentation/scripts-font-size-expected-mismatch.html: Added.