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] Refine when legacy/NG layout is forced #25522

Merged
merged 1 commit into from Sep 15, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

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

This is a refinement of [1] [2] [3] [4]:

  1. MathML elements should not try to force NG layout when using a
    non-math display.
  2. Really make ShouldForceNGLayout() override ShouldForceLegacyLayout()
    ([1] was only doing that for the inherited child context).
  3. Make MathML elements class keep handling the case of a forced legacy
    layout, but only on non-math display values. Note that this check is
    not necessary in LayoutObjectFactory::CreateMath which is only called
    for math display values.
  4. Allow to override the display of the <math> tag too (it's unclear
    why [3] had a special case and that does not match the spec).

Ideally, 3. can be removed when we completely switch to NG layout for
all displays. New tests are added to verify related crashes found by
clusterfuzz and check overriding of the layout MathML elements with
various display values.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2398702
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2397949
[3] https://chromium-review.googlesource.com/c/chromium/src/+/1985766
[4] https://chromium-review.googlesource.com/c/chromium/src/+/1917207

Bug: 6606, 1127628, 1127407, 1127222
Change-Id: I62b0ef4de623f4eb93184a1cecce598905160dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409957
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806771}

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.

This is a refinement of [1] [2] [3] [4]:

1. MathML elements should not try to force NG layout when using a
  non-math display.
2. Really make ShouldForceNGLayout() override ShouldForceLegacyLayout()
   ([1] was only doing that for the inherited child context).
3. Make MathML elements class keep handling the case of a forced legacy
  layout, but only on non-math display values. Note that this check is
  not necessary in LayoutObjectFactory::CreateMath which is only called
  for math display values.
4. Allow to override the display of the <math> tag too (it's unclear
   why [3] had a special case and that does not match the spec).

Ideally, 3. can be removed when we completely switch to NG layout for
all displays. New tests are added to verify related crashes found by
clusterfuzz and check overriding of the layout MathML elements with
various display values.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2398702
[2] https://chromium-review.googlesource.com/c/chromium/src/+/2397949
[3] https://chromium-review.googlesource.com/c/chromium/src/+/1985766
[4] https://chromium-review.googlesource.com/c/chromium/src/+/1917207

Bug: 6606, 1127628, 1127407, 1127222
Change-Id: I62b0ef4de623f4eb93184a1cecce598905160dad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409957
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806771}
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

3 participants