445509 - Make sure CSS text attributes are performant - use nsIFrame instead of computed style

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

In patch for bug 34759 Surkov is implementing text attributes. For now he uses GetComputedStyle(), which is fine for testing. However, Roc expressed concerned that GetComputedStyle() is very slow -- much slower than getting info from the frame. We should either just take his word for it or do some testing and possibly change to use the frame.

Comment 1

[alexander :surkov (:asurkov)]

(In reply to comment #0)
> In patch for bug 34759 Surkov is implementing text attributes. For now he uses
> GetComputedStyle(), which is fine for testing. However, Roc expressed concerned
> that GetComputedStyle() is very slow -- much slower than getting info from the
> frame. We should either just take his word for it or do some testing and
> possibly change to use the frame.
> 

GetComputedStyle() uses nsComputedDOMStyle that calculates styles on response and has complicated logic at the first glance. I don't get why this logic is surplus for us and how nsIFrame can help here.

Comment 2

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

I would do testing before changing anything, although you might find that getting information from the frame (e.g. via GetStyleText etc) actually gives simpler code.

OTOH I'm not 100% sure the accessibility module can get that data since you don't link to gklayout.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> I would do testing before changing anything, although you might find that
> getting information from the frame (e.g. via GetStyleText etc) actually gives
> simpler code.

nsComputedDOMStyle uses GetStyleText(). I just can't understand why the logic of nsComputedDOMStyle is overkill for us? Or is the code nsComputedDOMStyle not optimal? Or does creation of nsComputedDOMStyle takes long time/much memory? Where is a weak place of nsComputedDOMStyle?

Comment 4

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

It converts everything to an nsIDOMCSSValue XPCOM object, which involves memory allocation, and often converts values to strings which you have to then parse again to use. It's not a huge problem, just extra overhead and more work for you as the user of the API.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Status?  Maybe bug 467146 should wait on this?

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> Status?  Maybe bug 467146 should wait on this?

I think they aren't related. Sure if the current approach doesn't fit to this then you can use approach suggested here like we have for background-color.

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #0)
> In patch for bug 34759

This was actually bug 345759

Comment 8

[alexander :surkov (:asurkov)]

fixed by bug 728907