Closed Bug 1120313 Opened 7 years ago Closed 7 years ago

Nested ruby inside ruby annotation is not handled properly

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(1 file)

The current nsLineLayout cannot handle nested ruby inside ruby annotation properly. It causes crashes like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=5333869&repo=mozilla-inbound
Attached patch patchSplinter Review
Could you please review this patch first? It blocks bug 1055658 hence also bug 1055667.

I'm a bit worry about introducing a loop every time allocating and freeing per-frame data and per-span data. But as the loop is mostly shallow, and usually can be skipped completely, I guess it won't be a big problem. The other option is to add another member to the nsLineLayout for that. I'm not sure which is better.
Flags: needinfo?(dbaron)
Attachment #8547797 - Flags: review?(dbaron)
Comment on attachment 8547797 [details] [diff] [review]
patch

GetOutmostLineLayout should be called GetOutermostLineLayout or
GetOuterLineLayout.

And maybe call the local |lineLayout| variable in NewPerSpanData
outerLineLayout instead?  (Same in FreeFrame and FreeSpan and
NewPerFrameData.)

r=dbaron with that
Flags: needinfo?(dbaron)
Attachment #8547797 - Flags: review?(dbaron) → review+
(I think the loop is fine; deeply nested ruby is not something we should care about.)
https://hg.mozilla.org/mozilla-central/rev/c4553528cfdd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.