Bug 123204 - Combining vertical-rl writing mode, float: left and -webkit-text-combine: horizontal has unexpected result following r147588
Summary: Combining vertical-rl writing mode, float: left and -webkit-text-combine: hor...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-10-23 06:37 PDT by Antoine Quint
Modified: 2016-12-08 18:13 PST (History)
8 users (show)

See Also:


Attachments
Test case (455 bytes, text/html)
2013-10-23 06:37 PDT, Antoine Quint
no flags Details
Expected rendering of test case (39.84 KB, image/tiff)
2013-10-28 16:45 PDT, Antoine Quint
no flags Details
Test Case with longer text (573 bytes, text/html)
2013-11-02 03:40 PDT, Koji Ishii
no flags Details
Patch (225.48 KB, patch)
2013-11-19 21:34 PST, Yuki Sekiguchi
beidson: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2013-10-23 06:37:15 PDT
Created attachment 214954 [details]
Test case

See the attached test case, which started rendering differently following http://trac.webkit.org/changeset/147588, a fix for https://bugs.webkit.org/show_bug.cgi?id=105692.
Comment 1 Antoine Quint 2013-10-23 06:37:39 PDT
<rdar://problem/15297778>
Comment 2 Glenn Adams 2013-10-23 08:52:07 PDT
This appears to be a case where, due to the transition to and from tatechuyoko (horizontal within vertical) mode, the line breaking context across the transition boundary should be cleared, i.e., you want to use:

m_renderTextInfo.m_lineBreakIterator.resetPriorContext();

at the the transition boundary.
Comment 3 Koji Ishii 2013-10-27 10:34:23 PDT
(In reply to comment #2)
> This appears to be a case where, due to the transition to and from tatechuyoko (horizontal within vertical) mode, the line breaking context across the transition boundary should be cleared, i.e., you want to use:
> 
> m_renderTextInfo.m_lineBreakIterator.resetPriorContext();
> 
> at the the transition boundary.

The proposed fix here doesn't look correct; resetting line break context should not occur at inline element boundaries nor at before/after tatechuyoko. See:
# For line breaking before and after the composition,
# it is treated as a regular inline with its actual contents.
http://dev.w3.org/csswg/css-writing-modes/#text-combine-layout

I looked at the test case file, but from this bug description, the result and the expected is not clear to me.
Comment 4 Koji Ishii 2013-10-27 10:38:44 PDT
oops, hit Enter too early; I mean, I wonder maybe this is by design. Antoine, can you clarify your expectations? I can then comment more properly.
Comment 5 Antoine Quint 2013-10-28 16:45:25 PDT
Created attachment 215346 [details]
Expected rendering of test case

Attached a screenshot of my expectation, which matches what WebKit used to render up to r147588.
Comment 6 Koji Ishii 2013-10-30 17:49:04 PDT
hm, the original looks more natural, but there should be a break opportunity before "1". I guess I need to look into more, but I suspect there has been a bug in box layout, and bug 105692 fix was right, but it brings hidden bug up.

I'll look further later.
Comment 7 Koji Ishii 2013-10-31 15:04:05 PDT
I can't say exactly how and where, but my best guess is that intrinsic size calculation is incorrect when a vertical span contains a horizontal span, and Glenn's change made the bug clearer.

When tatechuyoko appears at the end of line, there should be break opportunities before and after. Exceptions are like when "!?" is in the tatechuyoko, because break before "!" is prohibited as per UAX#14.

I feel sorry that I guess this isn't really helpful advice...
Comment 8 Koji Ishii 2013-11-02 03:40:40 PDT
Created attachment 215806 [details]
Test Case with longer text
Comment 9 Koji Ishii 2013-11-02 03:45:49 PDT
See "Test Case with longer text"; this should not wrap, and is not related with bug 105692 (I suppose; I don't have older build.)

So a wild guess is that:
1. WebKit has a bug to calculate intrinsic size. A guess from the test case is that, it calculates character advances without writing-mode or tatechuyoko in mind. In the case of the attached test case, it might add *height* of "1", not *width*, and thus resulting box is smaller and lead to line break.
2. In the case of the attached test case, it does not break, there were no break opportunity.
3. With bug 105692 fixed, now the line has a break opportunity, and the original bug appear.

So, having a line break opportunity is correct, and we need to attack the original bug.
Comment 10 Yuki Sekiguchi 2013-11-19 21:34:10 PST
Created attachment 217381 [details]
Patch
Comment 11 Koji Ishii 2014-04-10 09:44:43 PDT
The patch from Yuki looks quite good to me. Can we get someone to review?
Comment 12 Brady Eidson 2016-05-24 22:05:22 PDT
Comment on attachment 217381 [details]
Patch

Assuming that patches for review since 2013 are stale, r-