51944 - remove mSelectorText from nsCSSSelector to reduce bloat

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Edward Kandrot]

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-4, no longer working on Mozilla)]

Yeah... I was commenting on this issue in bug 42953.

Comment 2

[Marc Attinasi]

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]

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-4, no longer working on Mozilla)]

See bug 42953 regarding other issues in this code.

Comment 5

[Marc Attinasi]

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]

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]

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]

marking verified per last comments

Comment 9

[Gerard Verron]

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]

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]

Marking verified per last comments.