If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

leafName returns a wrong string [PATCH]

RESOLVED FIXED in mozilla0.9.3

Status

()

Core
XPCOM
P3
major
RESOLVED FIXED
17 years ago
14 years ago

People

(Reporter: Pete Collins (MDG), Assigned: Pete Collins (MDG))

Tracking

Trunk
mozilla0.9.3
Other
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

(Assignee)

Description

17 years ago
leafName returns a wrong string after manipulation
from parent.

Output of Test:

js> o;
[xpconnect wrapped nsIFile @ 0x80972c8]
js> o.path;
/tmp
js> o.leafName;
tmp
js> o.path;
/tmp
js> o.parent.path;
/
js> o.leafName;
mp
js> o.path;
//mp
js>

You can assign this to me if you want and i'll take a look at it.

--pete

Comment 1

17 years ago
Does not appear to be an XPCOM problem to me from the discription.  Reassigning
as reuqested.  Thanks.
Assignee: kandrot → petejc

Comment 2

17 years ago
kandrot: Last I checked the implementations of nsIFile were in xpcom.
(Assignee)

Comment 3

17 years ago
Created attachment 39484 [details] [diff] [review]
patch
(Assignee)

Comment 4

17 years ago
OK, This patch fixes the problem

--pete
Keywords: patch, review
Summary: leafName returns a wrong string after manipulating the parent → leafName returns a wrong string [PATCH]
(Assignee)

Comment 5

17 years ago
Setting Target to 0.9.3

--pete
Target Milestone: --- → mozilla0.9.3
(Assignee)

Updated

17 years ago
Priority: -- → P3
(Assignee)

Updated

17 years ago
Severity: normal → major
(Assignee)

Comment 6

17 years ago
Test output after fix:


js> o;
[xpconnect wrapped nsIFile @ 0x814b678]
js> o.path;
/tmp
js> o.parent.path;
/
js> o.path;
/tmp
js> o.parent.path;
/
js> r;
[xpconnect wrapped nsIFile @ 0x814bcb0]
js> r.path;
/
js> r.parent.path;
/
js> r.path;
/
js> 
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 7

17 years ago
Where is |orig| freed?

nits:

Why did the whitespace change on @@ -929,7 +930,7 @@

Use PRBool instead of bool.

Even though I uses curly braces like you, the heathen owners don't:       

if (slashp == buffer)
-        slashp++;
+    {

Zap that space between the "if" and "else":
+
+    if(root)
+      mPath.Adopt(orig);
+
+    else
(Assignee)

Comment 8

17 years ago
Created attachment 39511 [details] [diff] [review]
changes per doug's comments
(Assignee)

Comment 9

17 years ago
Where is |orig| freed?

See path  #39511. . .

nits:

> Why did the whitespace change on @@ -929,7 +930,7 @@

This is a rsult of previous tabs that someone checked in. 

> Use PRBool instead of bool.

Yes, forgot. It in now.

> Even though I uses curly braces like you, the heathen owners don't:       

if (slashp == buffer)
-        slashp++;
+    {


The curly braces are to accomondate the assignment of `root=PR_TRUE'
I don't use curly braces for one liners either . . .  ;-)

> Zap that space between the "if" and "else":

Zapped  . . . 

--pete

Comment 10

17 years ago
nit: 
if thou brace "else", thou must not forsaken "if".  
spacing problem still is there (but I am not one to talk).

Fix that and r=dougt
(Assignee)

Comment 11

17 years ago
Created attachment 39512 [details] [diff] [review]
fix for 'return NS_ERROR_FAILURE' ws
(Assignee)

Comment 12

17 years ago
Created attachment 39514 [details] [diff] [review]
unforsaking the if . . .
(Assignee)

Comment 13

17 years ago
Created attachment 39516 [details] [diff] [review]
minor ws tweak
Looks good to me, but please change the brace style in the changes to match
what's done in the rest of thise file, i.e.:

    if (...) {
        ..
    }

and not:

    if (...)
    {
        ...
    }

and also use 4-space indentation, just like the other code in that file does.
Consistency with surrounding code overways correctness when it comes to
whitespace useage since there is no correct way! :-)

With that fixed, sr=jst
s/overways/overweighs/
(Assignee)

Comment 16

17 years ago
Created attachment 40267 [details] [diff] [review]
patch with jst's suggestions applied
(Assignee)

Comment 17

17 years ago
I checked this in this morning

resolving fixed

--pete
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.