Last Comment Bug 445509 - Make sure CSS text attributes are performant - use nsIFrame instead of computed style
: Make sure CSS text attributes are performant - use nsIFrame instead of comput...
Status: RESOLVED FIXED
: access, perf
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla13
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: textattra11y cleana11y a11yperf
  Show dependency treegraph
 
Reported: 2008-07-16 07:40 PDT by Aaron Leventhal
Modified: 2012-03-23 04:13 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Aaron Leventhal 2008-07-16 07:40:29 PDT
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 2008-07-22 00:59:16 PDT
(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) 2008-07-22 02:18:00 PDT
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 2008-07-22 02:40:21 PDT
(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) 2008-07-22 03:52:06 PDT
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] 2009-01-14 11:11:19 PST
Status?  Maybe bug 467146 should wait on this?
Comment 6 alexander :surkov 2009-01-14 22:03:51 PST
(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] 2009-11-30 12:47:58 PST
(In reply to comment #0)
> In patch for bug 34759

This was actually bug 345759
Comment 8 alexander :surkov 2012-03-23 04:13:19 PDT
fixed by bug 728907

Note You need to log in before you can comment on or make changes to this bug.