1175846 - lines of text-decoration across some elements are broken only in vertical-rl;

Status: RESOLVED DUPLICATE of bug 1220438

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Only in vertical-rl, lines of text-decoration isn't painted as connected lines. See following testcase:

http://jsfiddle.net/d_toybox/wbd7yr56/2/

In this testcase, I use lang attribute for changing the font-family. It seems that the font difference causes this bug.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Here? (in nsTextFrame::DrawTextRunAndDecorations())
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrame.cpp?rev=e6e2759a1205&mark=6333-6339#6325

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Or, around GetTextDecorations(). The method doesn't check if the frame is in VertualRL or VerticalLR.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Taking.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

I'm not sure why this patch fixes this bug. However, I guess you know since you wrote this.

Comment 5

[Jonathan Kew [:jfkthame]]

Comment on attachment 8624685 [details] [diff] [review]
Patch

Review of attachment 8624685 [details] [diff] [review]:
-----------------------------------------------------------------

TBH, I don't recall offhand exactly what's going on here, but I'm concerned that removing this code will adversely affect the visual overflow area computation for text frames. I notice that in the reftest here, I see clipping of the "j" descender when the patch is applied (on OS X; may be platform-dependent according to fonts used). Without the patch, this clipping doesn't happen.

So please take a closer look, especially at the effect on overflow/clipping area; also check the effect when text-orientation:sideways-right is applied, as I think that will make it clearer if ascent/descent metrics are being incorrectly flipped.

Leaving r? pending for now, as I think we need to look at this more deeply.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Jonathan Kew (:jfkthame) from comment #5)
> TBH, I don't recall offhand exactly what's going on here, but I'm concerned
> that removing this code will adversely affect the visual overflow area
> computation for text frames. I notice that in the reftest here, I see
> clipping of the "j" descender when the patch is applied (on OS X; may be
> platform-dependent according to fonts used). Without the patch, this
> clipping doesn't happen.

Hmm, without the patch, sometimes right-side line is clipped. So, I think that removing the code is not incorrect. But we see a hidden bug.

> So please take a closer look, especially at the effect on overflow/clipping
> area; also check the effect when text-orientation:sideways-right is applied,
> as I think that will make it clearer if ascent/descent metrics are being
> incorrectly flipped.

Okay, I will check that.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Hmm, applying text-orientation:sideways-right breaks underline/overline positions.
http://jsfiddle.net/d_toybox/wbd7yr56/5/

And also I don't see the clipping issue on Windows. I guess that it's really different bug in computing overflow area.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't have much time to work on this. Feel free to take this.

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

This bug seems to have been fixed by bug 1220438? Looks like the same issue so dupe for now. If you can still reproduce it, please reopen it.