Bug 181360 – Add expectation files of mathml/opentype/opentype-stretchy-horizontal.html for Windows

• Component: MathML
• Status: RESOLVED FIXED

[Minsheng Liu]

See

https://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r226096%20(3524)/retries/mathml/opentype/opentype-stretchy-horizontal-diffs.html

and

https://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r226096%20(3524)/mathml/opentype/opentype-stretchy-horizontal-pretty-diff.html

The logical widths are way off, and the glyph assembly also fails.

[Minsheng Liu]

Unfortunately I won't have access to a Windows PC until next weekend. What needs to be done in the meantime?

[Frédéric Wang (:fredw)]

(In reply to Minsheng Liu from comment #0)
> The logical widths are way off, and the glyph assembly also fails.

The width seems too small (at least the middle glyphs of the assembly are missing and the start & end glyphs are too close). The logical height is too large.

git log -- LayoutTests/platform/win/mathml/opentype/opentype-stretchy-horizontal-expected.txt

gives me the following history:

https://trac.webkit.org/changeset/169644
https://trac.webkit.org/changeset/202776
https://trac.webkit.org/changeset/202794
https://trac.webkit.org/changeset/203212
https://trac.webkit.org/changeset/225736

I am not sure the painting of stretchy operator has ever worked well on AppleWin (there are no PNG references in the history) and I think the test even randomly crashed in the past. The crazy logical height was already here when I first removed the failure expectation and added back the txt reference ( https://trac.webkit.org/changeset/202794 ). So I don't think new issues were caused by r225736.

So maybe just re-baseline the test with the current txt output for now.

Note: Personally, I won't spend too much time on the AppleWin port... I would even actually recommend you to get more freedom and try a nice operating system called Linux at some point :-) That will also allow you to update tests for the GTK port ;-)

[Minsheng Liu]

Created attachment 330638 [details]
Patch

[Minsheng Liu]

> I am not sure the painting of stretchy operator has ever worked well on
> AppleWin (there are no PNG references in the history) and I think the test
> even randomly crashed in the past. The crazy logical height was already here
> when I first removed the failure expectation and added back the txt
> reference ( https://trac.webkit.org/changeset/202794 ). So I don't think new
> issues were caused by r225736.

Glad to hear that.

> Note: Personally, I won't spend too much time on the AppleWin port... I
> would even actually recommend you to get more freedom and try a nice
> operating system called Linux at some point :-) That will also allow you to
> update tests for the GTK port ;-)

Sure. Personally, I find less choices (in tools) leads to more freedom (in action) but you know, I love open source. With all the other bugs we plan to fix, I believe testing the GTK port will become inevitable. I have some experience with old fashioned shell on Linux via ssh, but Linux with GUI is indeed new to me. Maybe I will love it some day :-)


(In reply to Frédéric Wang (:fredw) from comment #2)
> (In reply to Minsheng Liu from comment #0)
> > The logical widths are way off, and the glyph assembly also fails.
> 
> The width seems too small (at least the middle glyphs of the assembly are
> missing and the start & end glyphs are too close). The logical height is too
> large.
> 
> git log --
> LayoutTests/platform/win/mathml/opentype/opentype-stretchy-horizontal-
> expected.txt
> 
> gives me the following history:
> 
> https://trac.webkit.org/changeset/169644
> https://trac.webkit.org/changeset/202776
> https://trac.webkit.org/changeset/202794
> https://trac.webkit.org/changeset/203212
> https://trac.webkit.org/changeset/225736
> 
> I am not sure the painting of stretchy operator has ever worked well on
> AppleWin (there are no PNG references in the history) and I think the test
> even randomly crashed in the past. The crazy logical height was already here
> when I first removed the failure expectation and added back the txt
> reference ( https://trac.webkit.org/changeset/202794 ). So I don't think new
> issues were caused by r225736.
> 
> So maybe just re-baseline the test with the current txt output for now.
> 
> Note: Personally, I won't spend too much time on the AppleWin port... I
> would even actually recommend you to get more freedom and try a nice
> operating system called Linux at some point :-) That will also allow you to
> update tests for the GTK port ;-)

[Frédéric Wang (:fredw)]

(In reply to Minsheng Liu from comment #3)
> Created attachment 330638 [details]
> Patch

@Minsheng: It seems Apple already worked around the issue in bug 181346. If you really want this patch, you should consider updating LayoutTests/platform/win/TestExpectations. But maybe at the end it's better to do nothing than to upload "wrong" txt/png expectations...

[Minsheng Liu]

(In reply to Frédéric Wang (:fredw) from comment #5)
> (In reply to Minsheng Liu from comment #3)
> > Created attachment 330638 [details]
> > Patch
> 
> @Minsheng: It seems Apple already worked around the issue in bug 181346. If
> you really want this patch, you should consider updating
> LayoutTests/platform/win/TestExpectations. But maybe at the end it's better
> to do nothing than to upload "wrong" txt/png expectations...

I think if test expectations are marked as failing then we should be good here right? I do not like the idea of submitting a wrong expectation either. Let us move on other things!

[Minsheng Liu]

*** This bug has been marked as a duplicate of bug 181346 ***

[Frédéric Wang (:fredw)]

(In reply to Minsheng Liu from comment #6)
> I think if test expectations are marked as failing then we should be good
> here right? I do not like the idea of submitting a wrong expectation either.
> Let us move on other things!

Yes, I agree. Thanks for checking this.

[Radar WebKit Bug Importer]

<rdar://problem/36335180>

[Minsheng Liu]

@Darin: the bug is unnecessary as it is a duplicate of 181346. I closed it but forgot to cancel the review flag. Sorry about not making it clear earlier. I hope I have enough permission to set commit-queue- (and apparently so) so this does not get merged.

For more info, see comment 2/5/6.

[Darin Adler]

(In reply to Minsheng Liu from comment #10)
> I hope I have enough permission to set commit-queue- (and
> apparently so) so this does not get merged.

You do.