Bug 130455

Summary: Update some references for MathML pixels tests
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, buildbot, bunhere, cdumez, cfleizach, commit-queue, dbarton, gyuyoung.kim, gyuyoung.kim, mpakulavelrutka, mrobinson, ossy, rniwa, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 130345    
Attachments:
Description Flags
Patch V1 (updating TestExpectations)
none
Patch V2 (Updating TestExpectations and trying to fix markup errors in tests)
none
Patch V3
none
Patch V4
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Patch V5 cfleizach: review+

Description Frédéric Wang (:fredw) 2014-03-19 08:38:06 PDT
We should update the txt & png references for the following tests:

- mathml/presentation/mspace-children.html
- mathml/presentation/mo-stretch.html
- mathml/presentation/mo.xhtml
- mathml/presentation/roots.xhtml
- mathml/presentation/row.xhtml

The last three are currently marked FAIL in the main TestExpectation. The two first are marked FAIL for some platforms.
Comment 1 Frédéric Wang (:fredw) 2014-03-19 08:41:44 PDT
*** Bug 127482 has been marked as a duplicate of this bug. ***
Comment 2 Frédéric Wang (:fredw) 2014-03-19 08:50:38 PDT
Created attachment 227187 [details]
Patch V1 (updating TestExpectations)

Here are the failing tests whose ref must be updated.
Comment 3 Frédéric Wang (:fredw) 2014-03-19 10:20:41 PDT
So I just tried on GTK+:

LayoutTests/mathml/presentation/row.xhtml

- I'm not sure what is tested by the "x+1" but we have other tests for token elements.
- The test used green <div> inside <mtext> which is invalid (see bug 124128). They should be replaced with <mspace> or inline-block span. 
- The stretchy operators are already tested in mo-stretch and they are many other reftests for operator stretching.

=> So actually I think we can remove the whole test?

LayoutTests/mathml/presentation/roots.xhtml

- only some small spacing changes
- I already rewrote the last "don't wrap" test as a reftest LayoutTests/mathml/presentation/bug95015.html so it can be removed.

LayoutTests/mathml/presentation/mo.xhtml

- one test uses the invalid <div> in <mtext> (see above)
- it's not clear to me what the other tests verify, but I believe we already test mfrac/mo/mathsize elsewhere...

=> Should we remove that test?

For the other platforms, I believe it is safe to just update the references of mathml/presentation/mspace-children.html and mathml/presentation/mo-stretch.html.
Comment 4 Frédéric Wang (:fredw) 2014-03-19 10:38:58 PDT
Created attachment 227194 [details]
Patch V2 (Updating TestExpectations and trying to fix markup errors in tests)

Here is an updated patch, that tries to fix some errors in the tests.

It seems that setting background color on <mo> (but not on other token elements) is broken so I'll open a separate bug for that and write a reftest.
Comment 5 Frédéric Wang (:fredw) 2014-03-19 23:11:50 PDT
Created attachment 227257 [details]
Patch V3

OK, this patch removes the mo.xhtml and row.xhtml tests since features seem to be already tested elsewhere. I updated the references for GTK and tried to do so for EFL (but for some reason the png screenshots are empty for me). Could someone please use this patch to

- update the png & txt references for roots.xhtml/mspace-children.html/mo-stretch.html on Windows?
- check the txt references and update the png images for roots.xhtml/mspace-children.html/mo-stretch.html on EFL?
- update the png references for roots.xhtml/mspace-children.html/mo-stretch.html on Mac?

Thanks,
Comment 6 Frédéric Wang (:fredw) 2014-03-19 23:17:01 PDT
Created attachment 227259 [details]
Patch V4

I forgot the --binary flag...
Comment 7 Build Bot 2014-03-20 00:09:01 PDT
Comment on attachment 227259 [details]
Patch V4

Attachment 227259 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5848997226872832

New failing tests:
mathml/presentation/roots.xhtml
Comment 8 Build Bot 2014-03-20 00:09:08 PDT
Created attachment 227261 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Frédéric Wang (:fredw) 2014-03-20 00:13:06 PDT
Created attachment 227263 [details]
Patch V5

Update the roots-expected.txt on Mac...
Comment 10 chris fleizach 2014-03-20 08:45:55 PDT
(In reply to comment #9)
> Created an attachment (id=227263) [details]
> Patch V5
> 
> Update the roots-expected.txt on Mac...

Is this patch ready for review? or are you still waiting for image results for various platforms ( i see some empty images in this patch)
Comment 11 Frédéric Wang (:fredw) 2014-03-20 08:49:31 PDT
(In reply to comment #10)
> Is this patch ready for review? or are you still waiting for image results for various platforms ( i see some empty images in this patch)

I'm waiting txt/images results mentioned in comment 5, but can not generate them myself. And the EWS bots only run the tests on mac IIUC.
Comment 12 Frédéric Wang (:fredw) 2014-03-20 23:13:47 PDT
Should I wait that people update the references?

Or should I commit the changes and update the references afterwards, once I get the bot results?
Comment 13 chris fleizach 2014-03-21 05:38:32 PDT
(In reply to comment #12)
> Should I wait that people update the references?
> 
> Or should I commit the changes and update the references afterwards, once I get the bot results?

You should probably commit and then try to be vigilant in getting results quickly. I don't think we're going to get anyone to provide those results prior
Comment 14 Frédéric Wang (:fredw) 2014-03-21 06:03:03 PDT
Committed r166059: <http://trac.webkit.org/changeset/166059>
Comment 15 Frédéric Wang (:fredw) 2014-03-21 06:29:16 PDT
I think the Mac and EFL will pass. At the moment the Windows and EFL tests are crashing so I won't be able to get the MathML results until it is fixed.