New issue
Advanced search Search tips

Issue 692094 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

"hyphens: auto" sometimes results in wrong line-wrapping on Android

Reported by herr.er...@gmail.com, Feb 14 2017

Issue description

Example URL:
see attached file

Steps to reproduce the problem:
1. add the CSS rule "hyphens: auto" to some part of the page
2. sometimes, word wrapping is wrong, causing unnecessary overflow

What is the expected behavior?
words are hyphenated correctly without overflow

What went wrong?
wrong hyphenation/word wrap causing overflow

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 56.0.2924.87  Channel: stable
OS Version: 7.0
Flash Version: 

As an aside, most of the words are even not hyphenated on Android although the dictionary for language (German) seems to be installed. But some words are hyphenated.
No problem on OS X: correctly hyphenated, no overflow.

This is on Nexus 6 with most recent Android version and Chrome stable from Play Store. We have received some user screenshots from different devices too.

 
chrome-android-broken-hyphenation.html
1.6 KB View Download
screenshot-android.png
416 KB View Download
screenshot-macos.png
90.8 KB View Download
Owner: kojii@chromium.org
Status: Assigned (was: Unconfirmed)

Comment 2 by rtoy@chromium.org, Feb 14 2017

Components: -Blink Blink>Layout
kojii@ Please update the component if this is not a Blink>Layout issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd1e6431c28b8f3de55d580b66662859baf9d28b

commit dd1e6431c28b8f3de55d580b66662859baf9d28b
Author: kojii <kojii@chromium.org>
Date: Sun Feb 19 09:11:00 2017

Fix hyphenated words not to overflow on Android

This patch fixes to choose the correct hyphenation point when the word
has multiple hyphenation points on Android.

The line breaker computes the number of characters that can fit and
computes that last hyphenation point within the number. The number was
not used correctly in the Android hyphenation engine.

The fix is about 5 lines in |HyphenationMinikin::lastHyphenLocation|,
but the test isn't small. The hyphenation engine loads dictionaries
using a mojo service, but since the mojo service is not available in
unit tests, the test creates the engine from the dictionary file it
loaded.

BUG= 692094 

Review-Url: https://codereview.chromium.org/2692233005
Cr-Commit-Position: refs/heads/master@{#451519}

[modify] https://crrev.com/dd1e6431c28b8f3de55d580b66662859baf9d28b/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h
[modify] https://crrev.com/dd1e6431c28b8f3de55d580b66662859baf9d28b/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/dd1e6431c28b8f3de55d580b66662859baf9d28b/third_party/WebKit/Source/platform/text/HyphenationTest.cpp
[modify] https://crrev.com/dd1e6431c28b8f3de55d580b66662859baf9d28b/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
[add] https://crrev.com/dd1e6431c28b8f3de55d580b66662859baf9d28b/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.h

Comment 4 by kojii@chromium.org, Feb 20 2017

NextAction: 2017-02-23
Status: Fixed (was: Assigned)

Comment 5 by kojii@chromium.org, Feb 23 2017

Labels: Merge-Request-57
The fix is rather safe, despite the patch isn't small; later parts of the CL is for testing.

Impact, hard to say, not every is page has hyphenation enabled, but for such pages, Android looks worse than all other platforms in rather high possibility.
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 23 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 Deleted

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 23 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9397e81ac507fdd66d1eb316dae364e3cfbcffe

commit d9397e81ac507fdd66d1eb316dae364e3cfbcffe
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Feb 23 14:24:08 2017

Merge 2987: Fix hyphenated words not to overflow on Android

This patch fixes to choose the correct hyphenation point when the word
has multiple hyphenation points on Android.

The line breaker computes the number of characters that can fit and
computes that last hyphenation point within the number. The number was
not used correctly in the Android hyphenation engine.

The fix is about 5 lines in |HyphenationMinikin::lastHyphenLocation|,
but the test isn't small. The hyphenation engine loads dictionaries
using a mojo service, but since the mojo service is not available in
unit tests, the test creates the engine from the dictionary file it
loaded.

BUG= 692094 

Review-Url: https://codereview.chromium.org/2692233005
Cr-Commit-Position: refs/heads/master@{#451519}
(cherry picked from commit dd1e6431c28b8f3de55d580b66662859baf9d28b)

Review-Url: https://codereview.chromium.org/2713553007 .
Cr-Commit-Position: refs/branch-heads/2987@{#659}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/d9397e81ac507fdd66d1eb316dae364e3cfbcffe/third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h
[modify] https://crrev.com/d9397e81ac507fdd66d1eb316dae364e3cfbcffe/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/d9397e81ac507fdd66d1eb316dae364e3cfbcffe/third_party/WebKit/Source/platform/text/HyphenationTest.cpp
[modify] https://crrev.com/d9397e81ac507fdd66d1eb316dae364e3cfbcffe/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.cpp
[add] https://crrev.com/d9397e81ac507fdd66d1eb316dae364e3cfbcffe/third_party/WebKit/Source/platform/text/hyphenation/HyphenationMinikin.h

Comment 9 by kojii@chromium.org, Feb 23 2017

NextAction: ----

Sign in to add a comment