1120313 - Nested ruby inside ruby annotation is not handled properly

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 1

[Xidorn Quan [:xidorn] UTC+10]

Attachment: patch

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.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(I think the loop is fine; deeply nested ruby is not something we should care about.)

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=425d8f6c59d7

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c4553528cfdd