Closed
Bug 51944
Opened 24 years ago
Closed 23 years ago
remove mSelectorText from nsCSSSelector to reduce bloat
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
VERIFIED
FIXED
People
(Reporter: kandrot, Assigned: attinasi)
Details
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•24 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•24 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
Closed: 24 years ago
Resolution: --- → FIXED
See bug 42953 regarding other issues in this code.
Assignee | ||
Comment 5•24 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•24 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•24 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 9•23 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•23 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
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•