837765 - xul textbox should cache a textrun instead of repeatedly creating and discarding it

Status: NEW

(Core :: XUL, defect)

Description

[Jonathan Kew [:jfkthame]]

As noted in bug 834609 comment 10, the XUL textbox code (nsTextBoxFrame.cpp) repeatedly constructs (and destroys) a gfxTextRun for the same string.

This happens because it uses nsLayoutUtils and nsRenderingContext methods to measure and draw text, and these take the string and create a "throwaway" textrun to do one thing - measure width, get bounds, draw, etc. - so that each operation the textbox needs to do with its text leads to a separate gfxTextRun construction.

We could significantly reduce the amount of textrun creation we're doing if we make nsTextBoxFrame cache the gfxTextRun corresponding to its text.

A complication is that if the textbox contains mixed-direction bidi text, we can't create a single gfxTextRun corresponding to its content, as textruns are unidirectional. So then we'd need something more complex (like nsTextFrameThebes) - or we could fall back to the existing code in that case, and let this be an optimization for LTR-only elements.

Comment 1

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

One possibility is that we could replace nsTextBoxFrame with something that uses nsTextFrames wrapped in an anonymous block. AFAIK nsTextBoxFrame has only a couple of features that nsTextFrames-in-a-block doesn't have: access-key underlining and cropping in the center. Both of these could be added to nsTextFrame pretty easily. (Center cropping sounds super-hard for general inline text but we could hack it in for the case where there's only one textframe, i.e. single-direction text. I bet it's broken for mixed-direction XUL labels already.)

This is more work but a bunch less code in the end.

Comment 2

[Jonathan Kew [:jfkthame]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> One possibility is that we could replace nsTextBoxFrame with something that
> uses nsTextFrames wrapped in an anonymous block. AFAIK nsTextBoxFrame has
> only a couple of features that nsTextFrames-in-a-block doesn't have:
> access-key underlining and cropping in the center. Both of these could be
> added to nsTextFrame pretty easily. (Center cropping sounds super-hard for
> general inline text but we could hack it in for the case where there's only
> one textframe, i.e. single-direction text. I bet it's broken for
> mixed-direction XUL labels already.)
> 
> This is more work but a bunch less code in the end.

Yes, ideally we ought to share this code instead of having separate implementations of such similar things.

As for cropping (at start/end), when poking into nsTextBoxFrame it looked to me like the cropping implementation there is pretty broken anyway, for any complex script - I'd like to think that nsTextFrame would do a better job of those cases.

Comment 3

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

I think we could map start/end to text-overflow and get a good implementation "for free". It's only "center" that can't be done that way.

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> I think we could map start/end to text-overflow and get a good
> implementation "for free". It's only "center" that can't be done that way.

Before working on patch of bug 898984, I investigated text-overflow, and found that, although the single-value form can be mapped to "end" perfectly, the behavior of two-value form is totally different from "start". I am not sure whether it is the correct behavior for text-overflow, but it is obvious that there are many things to do before transforming the current implementation to using nsTextFrame.

Comment 5

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

We need to map in the other direction. XUL's crop=start and crop=end can be mapped to text-overflow easily, but crop=center can't.