remove mSelectorText from nsCSSSelector to reduce bloat

VERIFIED FIXED

Status

()

Core
Layout
P3
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Edward Kandrot, Assigned: Marc Attinasi)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

17 years ago
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.
Yeah... I was commenting on this issue in bug 42953.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

17 years ago
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?
Status: NEW → ASSIGNED
Target Milestone: --- → M19
(Reporter)

Comment 3

17 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
See bug 42953 regarding other issues in this code.
(Assignee)

Comment 5

17 years ago
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

17 years ago
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.
(Assignee)

Comment 7

17 years ago
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

17 years ago
marking verified per last comments
Status: RESOLVED → VERIFIED

Comment 9

16 years ago
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 
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

16 years ago
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.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago16 years ago
Resolution: --- → FIXED

Comment 11

16 years ago
Marking verified per last comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.