Closed Bug 975681 Opened 12 years ago Closed 12 years ago

Add tests for MathML mfrac

Categories

(Core :: MathML, defect, P5)

Product:
Core
Component:
MathML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla31

People

(Reporter: anujagarwal464, Assigned: anujagarwal464)

References

Details

Attachments

(2 files, 4 obsolete files)

mochitests for MathML mfrac
4.96 KB, patch
fwang
: review+
Details | Diff | Splinter Review
bug975681-reftests.diff
31.35 KB, patch
fwang
: review+
Details | Diff | Splinter Review
No description provided.
Blocks: 897065
Priority: -- → P5
Assignee: nobody → anujagarwal464
Attached patch bug975681.diff (obsolete) — — Splinter Review
Attachment #8395354 - Flags: feedback?(fred.wang)
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+
Attached patch bug975681-reftests.diff (obsolete) — — Splinter Review
I will add Mochitests in another patch.
Attachment #8395354 - Attachment is obsolete: true
Attachment #8397008 - Flags: feedback?(fred.wang)
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+
Summary: Add tests for mfrac → Add tests for MathML mfrac
Attached patch reftests for MathML mfrac (obsolete) — — Splinter Review
updated the patch.
Attachment #8397008 - Attachment is obsolete: true
Attachment #8397823 - Flags: review?(fred.wang)
Attached patch mochitests for MathML mfrac (obsolete) — — Splinter Review
Attachment #8397824 - Flags: review?(fred.wang)
Attachment #8397823 - Flags: review?(fred.wang) → review+
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+
updated tests. Thanks for help! :D
Attachment #8397824 - Attachment is obsolete: true
Attachment #8397864 - Flags: review?(fred.wang)
Attachment #8397864 - Flags: review?(fred.wang) → review+
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)
(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)
updated the mfrac-B-1 test.
Attachment #8397823 - Attachment is obsolete: true
Attachment #8398532 - Flags: review?(fred.wang)
Attachment #8398532 - Flags: review?(fred.wang) → review+
(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
https://hg.mozilla.org/mozilla-central/rev/742f05b9917b https://hg.mozilla.org/mozilla-central/rev/9d0ede143f7c
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
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.

Attachment

General

Creator:
Created:
Updated:
Size: