399159 - left edge is not aligned when text-align: justify; in Japanese paragraph

Status: RESOLVED FIXED

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Patch rv1.0

See the URL. It is a Japanese text justification, but the left edges are not aligned to a line. This might be a regression of new textframe.

We should drop the characters at start of lines from justifiable character.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  aIter->SetOriginalOffset(mStart.GetOriginalOffset());

You can just write *aIter = mStart.

+  // Ignore trailing cluster at end of line for justification purposes

This comment is wrong.

+  while (aIter->GetOriginalOffset() < mStart.GetOriginalOffset() + mLength-1) {

This could allow the start of the range to go past the end of the range. You probably should combine the two methods into FindJustificationRange and set up both iterators at the same time, and use the result of one iterator as the end-condition for the other.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Patch rv2.0

Thanks.

But I found another bug. If there are two Japanese characters in a line, the both characters are aligned to left. (Both characters are not in justifiable range...) However, it's very minor case, we should ignore it now...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  *aStart = mStart;
+  *aEnd = mStart;

Don't do this. Just assume that aStart and aEnd are set up to point to the right sort of iterator, and call SetOriginalOffset on them to set the right offsets. Then document that aStart and aEnd should be "near" the start and end of the frame's text in the textrun. It's more efficient this way, and it's what FindEndOfJustificationRange already does.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Patch rv2.1

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

checked-in.

-> FIXED

Comment 6

[:Gavin Sharp [email: [e-mail]]]

I backed this out because it was suspected of having caused the Tp regression that's been keeping the tree closed. See bug 400045 comment 9.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This could only affect pages that use text-align:justify a lot, and those very little. But it's worth a try I guess.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Numbers seem to show this was not the cause of any regression. We can reland this when the tree opens.

Comment 9

[Reed Loden [:reed]]

Masayuki, please reland this before Monday.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

checked-in.