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

[LayoutNG] Fix line breaking behavior after out-of-flow objects #18942

Merged
merged 2 commits into from Sep 10, 2019

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 9, 2019

This patch changes line breaking behavior after out-of-flow
objects to match to ICU, assuming the out-of-flow object is
an object replacement character.

The behavior is not well-defined, and not all cases are
interoperable across existing implementations. Legacy/WebKit
resets the prior context so that text after out-of-flow
objects behave as if it is at the top of the line. Gecko and
Edge seem to allow break in most cases, even when NBSP
follows, but not when the line overflows.

The behavior implemented in this patch is interoperable for
all tests that pass on existing 4 implementations, but not
exactly same as legacy when implementations do not agree.

Bug: 1001438
Change-Id: I4c7077d2c287a6e897175b29a73ce1c308f29f4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792467
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694888}

Copy link
Collaborator

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

Already reviewed downstream.

This patch changes line breaking behavior after out-of-flow
objects to match to ICU, assuming the out-of-flow object is
an object replacement character.

The behavior is not well-defined, and not all cases are
interoperable across existing implementations. Legacy/WebKit
resets the prior context so that text after out-of-flow
objects behave as if it is at the top of the line. Gecko and
Edge seem to allow break in most cases, even when NBSP
follows, but not when the line overflows.

The behavior implemented in this patch is interoperable for
all tests that pass on existing 4 implementations, but not
exactly same as legacy when implementations do not agree.

Bug: 1001438
Change-Id: I4c7077d2c287a6e897175b29a73ce1c308f29f4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792467
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694888}
@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 992be2f into master Sep 10, 2019
12 of 13 checks passed
12 of 13 checks passed
website-build-and-publish
Details
update-pr-preview
Details
.github/workflows/push-build-release-manifest.yml .github/workflows/push-build-release-manifest.yml
Details
Azure Pipelines Build #20190910.130 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-cl-1792467 branch Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.