Closed
Bug 975681
Opened 10 years ago
Closed 10 years ago
Add tests for MathML mfrac
Categories
(Core :: MathML, defect, P5)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: anujagarwal464, Assigned: anujagarwal464)
References
Details
Attachments
(2 files, 4 obsolete files)
4.96 KB,
patch
|
fredw
:
review+
|
Details | Diff | Splinter Review |
31.35 KB,
patch
|
fredw
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Updated•10 years ago
|
Priority: -- → P5
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → anujagarwal464
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8395354 -
Flags: feedback?(fred.wang)
Comment 2•10 years ago
|
||
Comment on attachment 8395354 [details] [diff] [review] bug975681.diff Review of attachment 8395354 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Anuj. Tests look good, but as I told you for menclose, using == tests is often more accurate than !=. Could you try to see if you could import and adapt some tests from https://github.com/mathjax/MathJax-test/tree/master/testsuite/MathMLToDisplay/Presentation/GeneralLayout/mfrac? I think they could replace many of the mfrac-A* tests. For dynamic tests, I guess you also want to test settting/adding attributes and adding/removing children, not just attribute removal. Could you also try to write Mochitests (see layout/mathml/tests) based on the MathML Acid3 (search mfrac in https://github.com/fred-wang/AcidTestsMathML/blob/master/acid3/index.html)?
Attachment #8395354 -
Flags: feedback?(fred.wang) → feedback+
Assignee | ||
Comment 3•10 years ago
|
||
I will add Mochitests in another patch.
Attachment #8395354 -
Attachment is obsolete: true
Attachment #8397008 -
Flags: feedback?(fred.wang)
Comment 4•10 years ago
|
||
Comment on attachment 8397008 [details] [diff] [review] bug975681-reftests.diff Review of attachment 8397008 [details] [diff] [review]: ----------------------------------------------------------------- Thank you, that looks very good. r=me with mfrac-A-6.html removed. > I will add Mochitests in another patch. Yes, please upload a separate patch with only Mochitests, so that will be more convenient for review. ::: layout/reftests/mathml/mfrac-A-6.html @@ +5,5 @@ > + </head> > + <body> > + <div style="position: absolute; left: 20px; top: 20px; width: 200px; height: 34px; background: red;"></div> > + </body> > +</html> I'm not sure this "34px" value is really reliable. I'd prefer a mochitest for that one.
Attachment #8397008 -
Flags: review+
Attachment #8397008 -
Flags: feedback?(fred.wang)
Attachment #8397008 -
Flags: feedback+
Assignee | ||
Updated•10 years ago
|
Summary: Add tests for mfrac → Add tests for MathML mfrac
Assignee | ||
Comment 5•10 years ago
|
||
updated the patch.
Attachment #8397008 -
Attachment is obsolete: true
Attachment #8397823 -
Flags: review?(fred.wang)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8397824 -
Flags: review?(fred.wang)
Updated•10 years ago
|
Attachment #8397823 -
Flags: review?(fred.wang) → review+
Comment 7•10 years ago
|
||
Comment on attachment 8397824 [details] [diff] [review] mochitests for MathML mfrac Review of attachment 8397824 [details] [diff] [review]: ----------------------------------------------------------------- Thank you! r=me with the comments below addressed. ::: layout/mathml/tests/test_bug975681.html @@ +24,5 @@ > + </p> > + > + <p> > + <math> > + <mfrac linethickness="2em" id="mfrac_linethickness"> I think this should be "px" rather than em so that it won't depend on the font-size. You should probably try a large enough thickness like "30px" so that it is larger than possible measurement errors. @@ +100,5 @@ > + var mfrac = document.getElementById("mfrac_linethickness").getBoundingClientRect(); > + var num = document.getElementById("mfrac_linethickness").firstElementChild.getBoundingClientRect(); > + var denom = document.getElementById("mfrac_linethickness").lastElementChild.getBoundingClientRect(); > + > + ok(almostLessThanRel(num.height + 2 + denom.height, mfrac.height) && almostLessThanAbs(num.bottom + 2, denom.top), "numerator and denominator should be separated by linethickness") I think you can just do almostLessThan(num.height + 30 + denom.height, mfrac.height) and almostLessThan(num.bottom + 30 , denom.top) The additional space between the bar and num/denum will even more help to pass the condition. Then the *Rel functions can be removed.
Attachment #8397824 -
Flags: review?(fred.wang)
Attachment #8397824 -
Flags: review+
Attachment #8397824 -
Flags: feedback+
Assignee | ||
Comment 8•10 years ago
|
||
updated tests. Thanks for help! :D
Attachment #8397824 -
Attachment is obsolete: true
Attachment #8397864 -
Flags: review?(fred.wang)
Updated•10 years ago
|
Attachment #8397864 -
Flags: review?(fred.wang) → review+
Assignee | ||
Comment 10•10 years ago
|
||
mfrac-C-1 and mfrac-D-1 are failing on OS X [max difference: 1, number of differing pixels: 1]. I think it would be OK to mark them fuzzy. mfrac-B-1 is failing on windows 7, don't know what to do with this test.
Flags: needinfo?(fred.wang)
Comment 11•10 years ago
|
||
(In reply to Anuj Agarwal [:anujagarwal464] from comment #10) > mfrac-C-1 and mfrac-D-1 are failing on OS X [max difference: 1, number of > differing pixels: 1]. I think it would be OK to mark them fuzzy. OK. > > mfrac-B-1 is failing on windows 7, don't know what to do with this test. when I open reftest analyzer, the "B" denominator seems to be slightly shifted. I think using <mspace/> elements for num/denum might help.
Flags: needinfo?(fred.wang)
Assignee | ||
Comment 12•10 years ago
|
||
updated the mfrac-B-1 test.
Attachment #8397823 -
Attachment is obsolete: true
Attachment #8398532 -
Flags: review?(fred.wang)
Updated•10 years ago
|
Attachment #8398532 -
Flags: review?(fred.wang) → review+
Comment 13•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=44222f065450
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Frédéric Wang (:fredw) from comment #13) > https://tbpl.mozilla.org/?tree=Try&rev=44222f065450 All test passed! :D
Keywords: checkin-needed
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/742f05b9917b https://hg.mozilla.org/integration/mozilla-inbound/rev/9d0ede143f7c
Flags: in-testsuite+
Keywords: checkin-needed
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/742f05b9917b https://hg.mozilla.org/mozilla-central/rev/9d0ede143f7c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 17•10 years ago
|
||
The following changesets are now in Firefox Nightly: > 742f05b9917b Bug 975681 - Mochitest for MathML mfrac. r=fredw > 9d0ede143f7c Bug 975681 - Reftests for MathML mfrac. r=fredw Nightly Build Information: ID: 20140402030201 Changeset: 4941a2ac0786109b08856738019b016a6c5a66a6 Version: 31.0a1 TBPL: https://tbpl.mozilla.org/?rev=4941a2ac0786 URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central Download Links: > Linux x86: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-i686.tar.bz2 > Linux x86_64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64.tar.bz2 > Linux x86_64 ASAN: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.linux-x86_64-asan.tar.bz2 > Mac: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.mac.dmg > Win32: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win32.installer.exe > Win64: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-02-03-02-01-mozilla-central/firefox-31.0a1.en-US.win64-x86_64.installer.exe Previous Nightly Build Information: ID: 20140401030203 Changeset: 1417d180a1d8665b1a91b897d1cc4cc31e7980d4 Version: 31.0a1 TBPL: https://tbpl.mozilla.org/?rev=1417d180a1d8 URL: https://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-01-03-02-03-mozilla-central
You need to log in
before you can comment on or make changes to this bug.
Description
•