Last Comment Bug 51944 - remove mSelectorText from nsCSSSelector to reduce bloat
: remove mSelectorText from nsCSSSelector to reduce bloat
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: ---
Assigned To: Marc Attinasi
: Chris Petersen
: Jet Villegas (:jet)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2000-09-08 17:48 PDT by Edward Kandrot
Modified: 2001-06-28 14:21 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Edward Kandrot 2000-09-08 17:48:34 PDT
mSelectorText is stored in the struct nsCSSSelector as a text string, but it is 
also contained in the "nsCSSSelector mSelector" field.  Looking at the output 
of waterson's new bloat snapshot tool, it appears to be using almost 100k, just 
to store this string.  Removing the clear text version will free up this memory 
at little cost.
Comment 1 David Baron :dbaron: ⌚️UTC-10 2000-09-10 07:04:04 PDT
Yeah... I was commenting on this issue in bug 42953.
Comment 2 Marc Attinasi 2000-09-12 10:19:38 PDT
It looks liek the code to serialze the selector already exists, mostly, in the 
List method. Maybe this can be formalized into another method for the DOM to 
call to get the selector when the actual selector text is removed?
Comment 3 Edward Kandrot 2000-09-12 17:13:20 PDT
I have a fix for it, and have had it checked over by Vidur.  I removed the
nsString for the text selector, and added a ToString function to nsCSSSelector. 
Getting the text now causes it to be generated by this new function.

The function is currently a dummy function, since I know little about how to
generate the string from the info in other fields, but will look into that based
on the comments here.  Even though it returns nothing now, it used to return
incorrect results, so it was approved for me to make this change, and to fill in
the ToString later this week.

The bloat is gone, and the functionality is as it was previously, so I am
closing this bug and will open one against the lack of functionality.
Comment 4 David Baron :dbaron: ⌚️UTC-10 2000-09-12 17:20:10 PDT
See bug 42953 regarding other issues in this code.
Comment 5 Marc Attinasi 2000-09-12 17:42:45 PDT
Edward, it is a good idea to get a review from the module owner, in this case 
Pierre or myself. Also, this was not an nsbeta3+ bug and we are suppossed to be 
only checking in fixes for P1 and P2 nsbeta3+ bugs at this time.

Anyway, the change looks fine - I wonder if the now empty methods should ASSERT 
so we can find the callers that are expecting them to actually do something and 
fix them as well - as it is, they will call SetSourceSelectorText and 
SetSelectorText and will have no idea that the calls are no-ops. Maybe add that 
to the new bug on implementing the ToString method? 

Thanks for taking this over and fixing it!
Comment 6 R. Saravanan 2000-09-20 15:54:41 PDT
Just commenting that the XMLterm component was using GetSelectorText through JS
DOM and it used to work correctly for changing styles interactively. See
http://lxr.mozilla.org/seamonkey/source/extensions/xmlterm/ui/content/XMLTermCommands.js#307
Now this feature is broken! I understand that getting rid of mSelectorText
reduces bloat. But it would be nice to be able to access the selectorText
property correctly through DOM as soon as possible.
Comment 7 Marc Attinasi 2000-09-20 16:13:38 PDT
R. Saravanan, Thanks for the note about the client use fo GetSelectorText. A new
bug was suppossed to be opened on the lack of functionality (see comment from
Edward Kandrot 2000-09-12 17:13) but I could not find it, so I just opened a new
one and CC'd you (bug 53448).
Comment 8 Chris Petersen 2001-03-05 13:48:19 PST
marking verified per last comments
Comment 9 Gerard Verron 2001-06-12 10:15:58 PDT
There is no way to access the cssRules, textSelector, and cssText properties 
from ns6. Even the styleSheets collection is not returned.

I get the following return:
uncaught exception: Exception..."Parameter is not an object" code:"1003" 
nsresult:"0+805303eb(NS_ERROR_DOM_NOT_OBJECT_ERR)" location:...

How can ns6 emulate styleSheets append or change on the fly. This is an 
important matter, and no documentation is available.

Thanks for the reply 
Comment 10 Marc Attinasi 2001-06-12 11:54:59 PDT
Thsi bug is fixed.

Gerard, please make your request for help in the mozilla.org newsgroups, or open
a new bug if you think that there is a defect. I recommend the
netscape.public.mozilla.dom  newsgroup on news.mozilla.org. Thanks you.
Comment 11 Chris Petersen 2001-06-28 14:21:34 PDT
Marking verified per last comments.

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