Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust MathML tests to Workaround WebKit's bug with document.fonts.ready #10025

Merged
merged 2 commits into from
Mar 14, 2018

Conversation

fred-wang
Copy link
Contributor

@fred-wang fred-wang commented Mar 14, 2018

cc @mrego cc @youennf

That seems to be an acceptable work around to me. WDYT?

See also https://bugs.webkit.org/show_bug.cgi?id=183628

@wpt-pr-bot
Copy link
Collaborator

There are no owners for this pull request besides its author. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@w3c-bots
Copy link

w3c-bots commented Mar 14, 2018

Build PASSED

Started: 2018-03-14 14:38:57
Finished: 2018-03-14 14:47:04

View more information about this build on:

@@ -62,7 +62,8 @@

setup({ explicit_done: true });
window.addEventListener("load", function() {
document.fonts.ready.then(runTests);
// Delay the check to workaround WebKit's bug https://webkit.org/b/174030.
setTimeout(() => { document.fonts.ready.then(runTests); }, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's usually preferred window.requestAnimationFrame() instead of setTimeout().
Could you try if something like that would fix the issue? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WebKit bug is that document.fonts.ready resolves immediately, even when I replace the CSS @font-face with a list of document.fonts.add(...) just before document.fonts.ready. Using setTimeout or requestAnimationFrame will delay things a bit and hence solve the issue, but I'm not sure requestAnimationFrame is more appropriate here, since it's not really a layout/painting issue but a font API bug. For example the following changes do not allow to workaround the issue:

document.fonts.ready.then(() => { requestAnimationFrame(runTests); });

or
document.fonts.ready.then(() => { setTimeout(runTests, notSmallEnoughDelta); });

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking in something like:

    window.requestAnimationFrame(() => { document.fonts.ready.then(runTests); });

However I agree with you that setTimeout() might be clearer, so I'm fine with keeping it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, requestAnimationFrame(() => { document.fonts.ready.then(runTests); }); would work too.

Copy link
Member

@mrego mrego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fred-wang fred-wang merged commit 716fa06 into master Mar 14, 2018
@fred-wang fred-wang deleted the mathml-fonts-ready branch March 14, 2018 14:48
hubot pushed a commit to WebKit/WebKit-http that referenced this pull request Mar 14, 2018
https://bugs.webkit.org/show_bug.cgi?id=183628

Patch by Frederic Wang <fwang@igalia.com> on 2018-03-14
Reviewed by Manuel Rego Casasnovas.

In r225162, the MathML tests from the WPT repository were imported. They were intended to
replace the ones in imported/mathml-in-html5 but the migration was not fully possible. The
main issue was that many MathML tests use Web fonts to test the use of OpenType parameters
but document.fonts.ready is unreliable in WebKit (bug 174030). A workaround was implemented
in WPT ( web-platform-tests/wpt#10025 ) so this commit finishes
the migration. We update the WPT MathML tests and now run them all, we remove
imported/mathml-in-html5 and the associated TestExpectations failures. Two cases unrelated to
font loading are still failing: One for the SuperscriptShiftUpCramped parameter (bug 156401)
and one for the RadicalDegreeBottomRaisePercent parameter (bug 183631).

LayoutTests/imported/w3c:

Reviewed by Manuel Rego Casasnovas.

* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-1-expected.txt.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-2-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-2-expected.txt.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-2.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/operators/mo-axis-height-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/radicals/root-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/radicals/root-parameters-1-expected.txt.
Also add the failure with RadicalDegreeBottomRaisePercent.
* web-platform-tests/mathml/presentation-markup/radicals/root-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/subsup-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1-expected.txt.
Also add the failure for SuperscriptShiftUpCramped.
* web-platform-tests/mathml/presentation-markup/scripts/subsup-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-2-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-2.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-3-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-3-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-3.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-4-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-4.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/tables/table-axis-height.html: Import font loading workaround.
* web-platform-tests/mathml/relations/css-styling/displaystyle-1.html: Import font loading workaround.
* web-platform-tests/mathml/relations/css-styling/lengths-3.html: Import font loading workaround.
* web-platform-tests/mathml/tools/utils/misc.py: Import update to Python 3.
(downloadWithProgressBar):

LayoutTests:

* TestExpectations: Unskip MathML WPT tests.
* imported/mathml-in-html5/LICENSE: Removed.
* imported/mathml-in-html5/README.md: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-2.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/radicals/root-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-3.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html: Removed.
* platform/mac-wk2/TestExpectations: Remove the expectation.
* platform/win/TestExpectations: Remove the expectation.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@229601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
fred-wang added a commit that referenced this pull request Mar 14, 2018
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=183628

Patch by Frederic Wang <fwang@igalia.com> on 2018-03-14
Reviewed by Manuel Rego Casasnovas.

In r225162, the MathML tests from the WPT repository were imported. They were intended to
replace the ones in imported/mathml-in-html5 but the migration was not fully possible. The
main issue was that many MathML tests use Web fonts to test the use of OpenType parameters
but document.fonts.ready is unreliable in WebKit (bug 174030). A workaround was implemented
in WPT ( web-platform-tests/wpt#10025 ) so this commit finishes
the migration. We update the WPT MathML tests and now run them all, we remove
imported/mathml-in-html5 and the associated TestExpectations failures. Two cases unrelated to
font loading are still failing: One for the SuperscriptShiftUpCramped parameter (bug 156401)
and one for the RadicalDegreeBottomRaisePercent parameter (bug 183631).

LayoutTests/imported/w3c:

Reviewed by Manuel Rego Casasnovas.

* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-1-expected.txt.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-2-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-2-expected.txt.
* web-platform-tests/mathml/presentation-markup/fractions/frac-parameters-2.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/operators/mo-axis-height-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/radicals/root-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/radicals/root-parameters-1-expected.txt.
Also add the failure with RadicalDegreeBottomRaisePercent.
* web-platform-tests/mathml/presentation-markup/radicals/root-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/subsup-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1-expected.txt.
Also add the failure for SuperscriptShiftUpCramped.
* web-platform-tests/mathml/presentation-markup/scripts/subsup-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-1-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-1.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-2-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-2.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-3-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-3-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-3.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-4-expected.txt: Renamed from LayoutTests/imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4-expected.txt.
* web-platform-tests/mathml/presentation-markup/scripts/underover-parameters-4.html: Import font loading workaround.
* web-platform-tests/mathml/presentation-markup/tables/table-axis-height.html: Import font loading workaround.
* web-platform-tests/mathml/relations/css-styling/displaystyle-1.html: Import font loading workaround.
* web-platform-tests/mathml/relations/css-styling/lengths-3.html: Import font loading workaround.
* web-platform-tests/mathml/tools/utils/misc.py: Import update to Python 3.
(downloadWithProgressBar):

LayoutTests:

* TestExpectations: Unskip MathML WPT tests.
* imported/mathml-in-html5/LICENSE: Removed.
* imported/mathml-in-html5/README.md: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/fractions/frac-parameters-2.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/radicals/root-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/subsup-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-1.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-2.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-3.html: Removed.
* imported/mathml-in-html5/mathml/presentation-markup/scripts/underover-parameters-4.html: Removed.
* platform/mac-wk2/TestExpectations: Remove the expectation.
* platform/win/TestExpectations: Remove the expectation.

Canonical link: https://commits.webkit.org/199283@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants