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

Revert "Initial support for text-decoration-line: spelling|grammar-error" #31774

Merged
merged 1 commit into from Nov 29, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

This reverts commit 643df2d079fac73cb420e6cd074c79d5a739f63d.

Reason for revert: text-decoration-line-grammar-error.html and text-decoration-line-spelling-error.html fail consistently on Mac10.12 Tests

Original change's description:

Initial support for text-decoration-line: spelling|grammar-error

This adds the initial support for spelling-error and grammar-error
values of text-decoration-line property.

Main changes are in TextDecorationInfo, where we need to support
the new values and paint the decoration correctly.
In Mac platform we use a dotted decoration to match
the platform conventions.
In other platforms we use a wavy decoration, but we modify it
so it looks similar to the spelling and grammar markers
in Microsoft Word.

It's important to note that this is just an intermediate step,
as this is different than how we currently paint
the spelling and grammar errors in DocumentMarkerPainter.
The idea would be make DocumentMarkerPainter use CSS text decorations
and then use this new code added here.

There are some known issues:

  • The underline offset is not the same than in Microsoft Word,
    but that's because of a problem with regular underlines
    not using the font metrics to compute that offset (crbug.com/1273042).
  • We should allow to tweak the color with text-decoration-color,
    added a TODO about that.

BUG=1163436

Change-Id: I117ad38fe3fc805619eb47b1df2f48c9d7c9a351
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3297885
Commit-Queue: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Delan Azabani <dazabani@igalia.com>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/main@{#945876}

Bug: 1163436
Change-Id: I2622bde37f2f8a7f725a1a06be8d34acdf9f728b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3306392
Auto-Submit: Greg Thompson <grt@chromium.org>
Owners-Override: Greg Thompson <grt@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#945903}

…ror"

This reverts commit 643df2d079fac73cb420e6cd074c79d5a739f63d.

Reason for revert: text-decoration-line-grammar-error.html and text-decoration-line-spelling-error.html fail consistently on Mac10.12 Tests

Original change's description:
> Initial support for text-decoration-line: spelling|grammar-error
>
> This adds the initial support for spelling-error and grammar-error
> values of text-decoration-line property.
>
> Main changes are in TextDecorationInfo, where we need to support
> the new values and paint the decoration correctly.
> In Mac platform we use a dotted decoration to match
> the platform conventions.
> In other platforms we use a wavy decoration, but we modify it
> so it looks similar to the spelling and grammar markers
> in Microsoft Word.
>
> It's important to note that this is just an intermediate step,
> as this is different than how we currently paint
> the spelling and grammar errors in DocumentMarkerPainter.
> The idea would be make DocumentMarkerPainter use CSS text decorations
> and then use this new code added here.
>
> There are some known issues:
> * The underline offset is not the same than in Microsoft Word,
>   but that's because of a problem with regular underlines
>   not using the font metrics to compute that offset (crbug.com/1273042).
> * We should allow to tweak the color with text-decoration-color,
>   added a TODO about that.
>
> BUG=1163436
>
> Change-Id: I117ad38fe3fc805619eb47b1df2f48c9d7c9a351
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3297885
> Commit-Queue: Delan Azabani <dazabani@igalia.com>
> Reviewed-by: Delan Azabani <dazabani@igalia.com>
> Reviewed-by: Stephen Chenney <schenney@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#945876}

Bug: 1163436
Change-Id: I2622bde37f2f8a7f725a1a06be8d34acdf9f728b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3306392
Auto-Submit: Greg Thompson <grt@chromium.org>
Owners-Override: Greg Thompson <grt@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#945903}
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.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 0b04431 into master Nov 29, 2021
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-145ee3b58a branch November 29, 2021 11:25
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