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

Fix ellipsis position for RTL elements #22380

Merged
merged 1 commit into from Mar 23, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 21, 2020

In ShapeResult::OffsetToFit() when the size to fit
matches result.origin_x we were returning
the left_character_index only if Direction() was RTL.
However if line_direction is RTL we also want that index.

With this change css/css-ui/text-overflow-028.html starts to pass
on Linus and Windows, we add new tests here too
(the RTL ones were failing before this patch).

BUG=1063319
TEST=css/css-overflow/text-overflow-ellipsis-001.html
TEST=css/css-overflow/text-overflow-ellipsis-rtl-001.html
TEST=css/css-overflow/text-overflow-ellipsis-vertical-001.html
TEST=css/css-overflow/text-overflow-ellipsis-vertical-ltr-001.html

Change-Id: Idbaed07a63d4ec6335e722b7fb7b4e91f4076564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111152
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#752380}

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.

In ShapeResult::OffsetToFit() when the size to fit
matches result.origin_x we were returning
the left_character_index only if Direction() was RTL.
However if line_direction is RTL we also want that index.

With this change css/css-ui/text-overflow-028.html starts to pass
on Linus and Windows, we add new tests here too
(the RTL ones were failing before this patch).

BUG=1063319
TEST=css/css-overflow/text-overflow-ellipsis-001.html
TEST=css/css-overflow/text-overflow-ellipsis-rtl-001.html
TEST=css/css-overflow/text-overflow-ellipsis-vertical-001.html
TEST=css/css-overflow/text-overflow-ellipsis-vertical-ltr-001.html

Change-Id: Idbaed07a63d4ec6335e722b7fb7b4e91f4076564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111152
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#752380}
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