User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0 Build ID: 20171024165158 Steps to reproduce: 1. Open https://www.heise.de/newsticker/meldung/US-Regierung-legt-Plan-fuer-verbindliche-Auto-zu-Auto-Kommunikation-auf-Eis-3877728.html 2. Scroll down to "lieber für breitbandiges mobiles Internet nutzen" Actual results: m-obiles Expected results: Correct hyphenation instead of random word-tearing would be nice. I've seen single teared-off consonants more often recently. Always in German text.

In the above testcase, see hyphenation of "V-ehicle" at the end of the third-from-last line.

This regressed in March with the landing of bug 1056516. A key feature of the testcase here is that there's an inline element within the paragraph (there are <a> links in the original webpage; I just used an <u> element in my testcase) that uses the *same* font as the rest of the text -- i.e. underlining or color change is fine, but it is *not* bold or italic -- so that it does not interrupt textrun generation. Jeremy, could you look into this? I'd guess there's an offset that needs fixing up somewhere...

(In reply to Jonathan Kew (:jfkthame) from comment #3) > This regressed in March with the landing of bug 1056516. > > A key feature of the testcase here is that there's an inline element within > the paragraph (there are <a> links in the original webpage; I just used an > <u> element in my testcase) that uses the *same* font as the rest of the > text -- i.e. underlining or color change is fine, but it is *not* bold or > italic -- so that it does not interrupt textrun generation. > > Jeremy, could you look into this? I'd guess there's an offset that needs > fixing up somewhere... Hmm... indeed, once we remove the inline tag (i.e., <u>), the bug is gone. I'll investigate this next week.

Let's evaluate an uplift once we have a fix.

I know what happened here. If we put more than one inline tags in a paragraph, they will still belongs to the same gfxTextRun, but might have more than one nsTextFragment, which is not taken into consideration while computing hyphenation data in [1]. For example: <p>test test <u>underline</u> test</p> This will create 3 nsTextFragments, one for the texts before <u> element, one for <u> element, and one for the texts after <u> element. The current implementation only considers one nsTextFragment per gfxTextRun. [] https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/layout/generic/nsTextFrame.cpp#3682-3683

try w/ test only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=82f2fbebd27e2fbe9b1a7287c0a0b13d5fe39c6e try w/ test and the fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=39641c3cdf164e47b04f41029a08cc9a2f8e3d46

Comment on attachment 8928936 [details] Bug 1415581 - get the correct positions of explicit hyphens while calling PropertyProvider::GetHyphenationBreaks. https://reviewboard.mozilla.org/r/200282/#review205986

Comment on attachment 8928957 [details] Bug 1415581 - add reftest. https://reviewboard.mozilla.org/r/200298/#review205988

Pushed by jichen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8d16bdf7d0fc add reftest. r=jfkthame https://hg.mozilla.org/integration/autoland/rev/ef9858e9ad38 get the correct positions of explicit hyphens while calling PropertyProvider::GetHyphenationBreaks. r=jfkthame

https://hg.mozilla.org/mozilla-central/rev/8d16bdf7d0fc https://hg.mozilla.org/mozilla-central/rev/ef9858e9ad38

(In reply to Jet Villegas (:jet) from comment #5) > Let's evaluate an uplift once we have a fix. The random line breaks are pretty annoying, and this issue is more easily presented in long paragraph articles (e.g., News, Blogs, ... etc.), which is making it even worse. It would be better that our users could get this fix sooner.

Comment on attachment 8928936 [details] Bug 1415581 - get the correct positions of explicit hyphens while calling PropertyProvider::GetHyphenationBreaks. Approval Request Comment [Feature/Bug causing the regression]: CSS hyphens property [User impact if declined]: texts with long paragraph may have some random/unexpected hyphen breaks [Is this code covered by automated tests?]: yes, a reftest is added in the patchset [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: no [Is the change risky?]: Not that I’m aware of [Why is the change risky/not risky?]: the change is to correct a index calculation, only the rendering of the line breaks is affected, so the risk should be low. [String changes made/needed]:

Comment on attachment 8928936 [details] Bug 1415581 - get the correct positions of explicit hyphens while calling PropertyProvider::GetHyphenationBreaks. Fix an annoying random line breaks issue. Beta58+.

https://hg.mozilla.org/releases/mozilla-beta/rev/6a0f1693d664 https://hg.mozilla.org/releases/mozilla-beta/rev/63888c62a793

(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #18) > [Is this code covered by automated tests?]: yes, a reftest is added in the > patchset > [Needs manual test from QE? If yes, steps to reproduce]: no Since this change has automated coverage and based on Jeremy's assessment on manual testing needs, marking this as qe-verify-.