Make data members of nsStr protected and clean up code

RESOLVED FIXED in mozilla0.9.7

Status

()

RESOLVED FIXED
18 years ago
17 years ago

People

(Reporter: jag+mozilla, Assigned: jag+mozilla)

Tracking

Trunk
mozilla0.9.7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

18 years ago
More code cleanup.
(Assignee)

Comment 1

18 years ago
Needs SubsumeStr patch in bug 98153.
Status: NEW → ASSIGNED
Depends on: 98153
Target Milestone: --- → mozilla0.9.5
(Assignee)

Comment 2

18 years ago
Created attachment 48189 [details] [diff] [review]
Make nsStr data members protected
Is the comment in nsDirectoryViewer.cpp no longer true?  If so, take it out.  If
not, could you find a way to do what the original code did?
(Assignee)

Updated

18 years ago
Attachment #48189 - Attachment is obsolete: true
(Assignee)

Comment 4

18 years ago
Created attachment 48687 [details] [diff] [review]
Updated per dbaron's comment
Could you explain why the SetLength calls still do what the old code did even
though SetLength does stuff other than set mLength?
(Assignee)

Comment 6

18 years ago
Sure. SetLength will grow the string if you're setting it to a length greater
than its current capacity. In this case it's a no-op since the code already did
that directly through SetCapacity a few lines up.

Comment 7

18 years ago
Mass-moving lower-priority 0.9.5 bugs off to 0.9.6 to make way for remaining
0.9.4/eMojo bugs, and MachV planning, performance and feature work. If you
disagree with any of these targets, please let me know.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
(Assignee)

Comment 8

17 years ago
-> 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 9

17 years ago
I've got similar work over in bug 108962.. jag and I discussed that he would try
to get the best of both worlds landed for this bug first, then I would submit
new patches for that bug.
Blocks: 108962

Comment 10

17 years ago
Comment on attachment 48687 [details] [diff] [review]
Updated per dbaron's comment

that said, sr=alecf
Attachment #48687 - Flags: superreview+
(Assignee)

Comment 11

17 years ago
Created attachment 57453 [details] [diff] [review]
Updated patch, compiles on linux, windows, mac
Is there a better solution for the MathML change?

I prefer Truncate(0) (or Truncate()) over SetLength(0), etc (when it is a
truncation).

What's the ToNewUTF8String doing there?
(Assignee)

Comment 13

17 years ago
I could remove the Operator constructor since no one uses it ;-)

Other than that, I could maybe convert this to use nsDependentString, let me
look at that if you think it's really an issue.

SetLength(0) fixed, and removed the explicit 0 terminations right above them
(and other SetLength calls in the same file) since SetLength takes care of that.

ToNewUTF8String pulls some tricks to avoid an extra copy if the
NS_ConvertUCS2toUTF8 allocated memory to do the conversion:

http://lxr.mozilla.org/seamonkey/source/string/src/nsReadableUtils.cpp#214
(Assignee)

Comment 14

17 years ago
Created attachment 57464 [details] [diff] [review]
Addressing most of the comments, will try not to copy in the mathml code if pressed.

alecf, sorry about not having it in yet.
(Assignee)

Comment 15

17 years ago
Created attachment 57573 [details] [diff] [review]
Above + removing the unused constructor of OperatorData.
Attachment #48687 - Attachment is obsolete: true
Attachment #57453 - Attachment is obsolete: true
Attachment #57464 - Attachment is obsolete: true

Comment 16

17 years ago
Comment on attachment 57573 [details] [diff] [review]
Above + removing the unused constructor of OperatorData.

so awesome. sr=alecf
Oh well, I was out of town for the sr= anyway :)
Attachment #57573 - Flags: superreview+
(Assignee)

Comment 18

17 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.