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

[mathml] Make math display values on pseudo elements compute to flow #25639

Merged
merged 2 commits into from Sep 22, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 19, 2020

This is a follow-up of [1] which didn't handle the case of pseudo
elements with a math display value. A reftest is added to verify the
expected behavior with pseudo elements, but will fail until the
parsing of math display values is adjusted to match what the CSSWG
agreed. However, this test checks the fix for an assertion failure
(http://crbug.com/1130127).

Bug: 6606, 1127222, 1130127

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2404789

Change-Id: I4f7c13db003f776045a934eae9be5839b3c0343f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418658
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Cr-Commit-Position: refs/heads/master@{#809177}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

Copy link

@nagelmodel nagelmodel left a comment

Choose a reason for hiding this comment

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

I think this could be related to Jerry Mathers leave it to beaver or Marshall Mathers the singer

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2418658 branch 2 times, most recently from 1cbb71d to 675577d Compare September 21, 2020 16:35
This is a follow-up of [1] which didn't handle the case of pseudo
elements with a math display value. A reftest is added to verify the
expected behavior with pseudo elements, but will fail until the
parsing of math display values is adjusted to match what the CSSWG
agreed. However, this test checks the fix for an assertion failure
(http://crbug.com/1130127).

Bug: 6606, 1127222, 1130127

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2404789

Change-Id: I4f7c13db003f776045a934eae9be5839b3c0343f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418658
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Cr-Commit-Position: refs/heads/master@{#809177}
The stability checks failed because the existing code resulting in
multiple test names with the same name. Fix this by using the class
attribute instead.

data-expected is from the /resources/check-layout-th.js framework.

Also use `element.localName` to get the correct capitalization.
@foolip
Copy link
Member

foolip commented Sep 22, 2020

@fred-wang this has gotten stuck in export because the subtest names aren't unique. That's because there was no class attribute, which the test names were trying to use. I've pushed a commit which I think will fix it, but can you review to verify I haven't just broken your test?

Copy link
Contributor

@fred-wang fred-wang left a comment

Choose a reason for hiding this comment

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

Thanks LGTM, let's land this.

The tests will likely need to be updated later after CSSWG discussions.

@foolip
Copy link
Member

foolip commented Sep 22, 2020

Oops, I didn't realize that @chromium-wpt-export-bot would merge this automatically after my fix. I'm glad it wasn't objectionable, then!

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.

None yet

6 participants