1140264 - Line is incorrectly broken between ruby and previous open bracket

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Xidorn Quan [:xidorn] UTC+11]

I see this bug in tweet [1]. This bug seems to happen in some condition.

[1] https://twitter.com/Takenspc/status/573652668582776832

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: testcase

This is a testcase simplified from that page. This bug can be reproduced when the window is resized to a certain width.

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Attachment: screenshot

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 1 - Avoid expanding auto-hidden ruby annotations

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 2 - move & merge line breaking code in rbc

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 3 - exclude rbc from inline descendant of ruby

The patch 2 actually fixes this bug, but causes regression when ruby is the the first element in a line. This patch fixes this problem by making rbc not inline descendant of ruby, which allows line layout to record NeedsBackup for <rb>s in CanPlaceFrame instead of skip them because of NoWrap.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 4 - reftest

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8574464 [details] [diff] [review]
patch 3 - exclude rbc from inline descendant of ruby

Maybe the flag should be renamed to IsNonbreakableDescendantOfRuby / IS_NONBREAKABLE_DESCENDANT_OF_RUBY or similar, since its meaning is now more subtle?

r=dbaron with that or similar

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8574465 [details] [diff] [review]
patch 4 - reftest

If this test relies on particular font size ratios between the ruby bases and ruby annotations, I think it should specify font sizes explicitly.

r=dbaron with that

Comment 9

[Xidorn Quan [:xidorn] UTC+11]

(In reply to David Baron [:dbaron] (UTC-8) from comment #8)
> Comment on attachment 8574465 [details] [diff] [review]
> patch 4 - reftest
> 
> If this test relies on particular font size ratios between the ruby bases
> and ruby annotations, I think it should specify font sizes explicitly.

I don't think it relies on that, because I use the unit rem in the annotation.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Ah, right.  It does rely on the font providing a square glyph for 咲.  I'm not sure how likely that is to break...

Comment 11

[Xidorn Quan [:xidorn] UTC+11]

Is there any test font which guarantees to generate a square for any cjk ideographic character? It seems Ahem doesn't include those characters.

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

Attachment: patch 3 - rename the style context bit and exclude rbc/rtc from it

Comment 13

[Xidorn Quan [:xidorn] UTC+11]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e10feab2b11

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e10feab2b11

Comment 15

[Simon Montagu :smontagu]

(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #11)
> Is there any test font which guarantees to generate a square for any cjk
> ideographic character? It seems Ahem doesn't include those characters.

http://blogs.adobe.com/CCJKType/2013/05/css-orientation-test-opentype-fonts.html sounds like what you're looking for (I haven't yet tried to use it myself).

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8575038 [details] [diff] [review]
patch 3 - rename the style context bit and exclude rbc/rtc from it

>Bug 1140264 part 3 - Rename IsInlineDescendantOfRuby to ShouldSUppressLineBreak and exclude rbc/rtc from it.

"SUppress" -> "Suppress"

>+  // the level containers themselvs are breakable. We have to check

themselvs -> themselves

>+  // because any of the ruby box could be anonymous.

box -> boxes


r=dbaron with that

Comment 17

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Simon Montagu :smontagu from comment #15)
> (In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #11)
> > Is there any test font which guarantees to generate a square for any cjk
> > ideographic character? It seems Ahem doesn't include those characters.
> 
> http://blogs.adobe.com/CCJKType/2013/05/css-orientation-test-opentype-fonts.
> html sounds like what you're looking for (I haven't yet tried to use it
> myself).

Thanks. I guess I can just use a latin character here with Ahem font, since the line breaking behavior here should be identical for latin character and ideographic character.

Comment 18

[Xidorn Quan [:xidorn] UTC+11]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ee3e7424d3a
https://hg.mozilla.org/integration/mozilla-inbound/rev/a71446331ce6
https://hg.mozilla.org/integration/mozilla-inbound/rev/f672b2cd7a4f
https://hg.mozilla.org/integration/mozilla-inbound/rev/144ea3057b66

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ee3e7424d3a
https://hg.mozilla.org/mozilla-central/rev/a71446331ce6
https://hg.mozilla.org/mozilla-central/rev/f672b2cd7a4f
https://hg.mozilla.org/mozilla-central/rev/144ea3057b66