Closed Bug 1081770 Opened 10 years ago Closed 10 years ago

Sync ruby annotations when base is justified

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently ruby annotation might not be synced with its base when the line is justified.
OS: Mac OS X → All
Hardware: x86 → All
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.
Depends on: ruby-align
Blocks: 1103832
Depends on: 1107721
Depends on: 1108429
No longer depends on: ruby-align
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → quanxunzhen
Attachment #8535273 - Flags: review?(dbaron)
Blocks: 1108429
No longer depends on: 1108429
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
Attachment #8535273 - Flags: review?(dbaron) → review+
(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?
Attached patch patchSplinter Review
This patch additionally considers the case that writing-mode of annotation is different from that of the base line.
Attachment #8535273 - Attachment is obsolete: true
Attachment #8536935 - Flags: review?(dbaron)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: