Currently ruby annotation might not be synced with its base when the line is justified.

We might finally need to move some of the reflow code into nsLineLayout, so that the line layout which reflows base frames could be aware of the text frames, and make it possible to sync them when TextAlignLine happens. We could investigate it later, but I do think it is the other hard part for ruby. The current behavior of WebKit and Blink is that, rubies are not justified at all, which I don't think is correct.

The way I plan to solve this is to make the line layout for annotations allocate PerFrameData and PerSpanData from the line layout of the base, and link the data together, so that ApplyFrameJustification is enabled to adjust the data of annotations as well. I'm going to fix it after bug 1055676.

Comment on attachment 8535273 [details] [diff] [review] patch This should have a commit message before being posted for review. I'd suggest something like: >Bug 1081770 - Move ruby annotation frames when text-align: justify is applied to ruby bases. r=dbaron SyncRectOfAnnotationFrames doesn't seem like the best name; "Sync Rect" is rather generic (and this code is just the horizontal aspects of applying text-align: justify). I'd suggest instead calling it ApplyJustificationToAnnotations. I'd also suggest maybe naming aSpan aContainingSpan, and adding a comment saying that aPFD must be one of the frames of aContainingSpan. >+ PerFrameData* pfd = aPFD->mNextAnnotation; Is there a bit you can use to assert that aPFD isn't an annotation? If so (and I thought there was), please add such an assertion. >+ // It is possible that there are siblings which do not attached to >+ // a ruby base-level frame. Their size will not be affected by the >+ // justification, but we still have to move them so that they won't >+ // overlap us. Why does this happen? Is it because of things like 'direction' changes within the ruby? And what guarantees that this doesn't affect things that are also in some other ruby base's mAnnotations? The comment should (briefly, at least) answer these questions. >+ inline void SyncRectOfAnnotationFrames(PerFrameData* aPFD, Drop the "inline". r=dbaron with that

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #4) > Comment on attachment 8535273 [details] [diff] [review] > patch > > This should have a commit message before being posted for review. I'd > suggest something like: > >Bug 1081770 - Move ruby annotation frames when text-align: justify is applied to ruby bases. r=dbaron > > SyncRectOfAnnotationFrames doesn't seem like the best name; "Sync Rect" > is rather generic (and this code is just the horizontal aspects of > applying text-align: justify). I'd suggest instead calling it > ApplyJustificationToAnnotations. This patch doesn't actually apply any justification to annotations. It just sync the position. So this name might be more confusing. Annotations do have justification when ruby-align is applied (which I have implemented in my local repo for bug 1108429). Maybe SyncJustificationToAnnotations. > >+ // It is possible that there are siblings which do not attached to > >+ // a ruby base-level frame. Their size will not be affected by the > >+ // justification, but we still have to move them so that they won't > >+ // overlap us. > > Why does this happen? Is it because of things like 'direction' changes > within the ruby? And what guarantees that this doesn't affect things > that are also in some other ruby base's mAnnotations? > > The comment should (briefly, at least) answer these questions. This happens in two cases: 1. intra-annotation whitespace not paired with intra-base whitespace (see bug 1099807); 2. extra annotations which do not have base to be paired with. I'll add comment.

Oh, I forgot that ruby-align had its own justification option. Maybe ApplyLineJustificationToAnnotations?

This patch additionally considers the case that writing-mode of annotation is different from that of the base line.

try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=96a60a9d2f91

Comment on attachment 8536935 [details] [diff] [review] patch r=dbaron

inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/cffd89b920e3