129054 - saving files to root dir doesn't save related items

Status: RESOLVED INVALID

(SeaMonkey :: Composer, defect)

Description

[Kathleen :Brade]

The composer saving code has a bug in it:
it does:  parentDir = tempLocalFile.parent;
However, nsILocalFile's "parent" is null if it's at the root.
If this is the case we should make an nsIURI that points to the root so related
files are saved there as intended.

Comment 1

[Boris Zbarsky [:bzbarsky]]

nsILocalFile's parent _should_ be null at the root.  See bug 121489

Comment 2

[sujay]

heads up charley and Tucson

Comment 3

[Kathleen :Brade]

this is a huge inconsistency, we should try to fix for 1.0

Comment 4

[Syd Logan]

Doesn't have anything to do with core editing of content, EDITORBASE-. But
marking nsbeta1+ since this is a large defect.

Comment 5

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 6

[Kathleen :Brade]

pushing off

Comment 7

[Kathleen :Brade]

seek reconsideration

Comment 8

[Samir Gehani]

Composer triage team: nsbeta1+/adt2

Comment 9

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 10

[Samir Gehani]

Editor triage team: nsbeta1+/adt3

Comment 11

[Kathleen :Brade]

Attachment: patch to test on windows

Comment 12

[Rafael Ebron (:rebron)]

Composer triage: nsbeta1-

Comment 13

[Kathleen :Brade]

I can't reproduce this problem so pushing off to 1.5a (although I think we still
want the patch above)

Comment 14

[Kathleen :Brade]

Comment on attachment 118400 [details] [diff] [review]
patch to test on windows

I'd like to land this patch (without the dumps) for 1.5a.  Requesting reviews
despite called code not working as documented.

Comment 15

[[e-mail]]

Comment on attachment 118400 [details] [diff] [review]
patch to test on windows

>             relatedFilesDir = tempLocalFile.parent;  // this is wrong if parent is the root!
>+            // if parent is root dir, we'll have null relatedFilesDir and
>+            // that is not what we want, we want to save files to root
>+            if (!relatedFilesDir)
>+              relatedFilesDir.path = relatedFilesDirStr;

I don't understand this bug. I don't see how you can have a problem getting the
parent of a file (which surely tempLocalFile must be, if you are trying to save
to it). But the fix is wrong anyway, because you're trying to set the .path of
null.

Comment 16

[Kathleen :Brade]

Attachment: remove bogus comment

Neil and I agree that removing the bogus comment is the correct thing here and
that this bug should be resolved as invalid.

Comment 17

[Kathleen :Brade]

Comment on attachment 124564 [details] [diff] [review]
remove bogus comment

r=Neil (via irc)
requesting jag's sr= for this comment-only patch.

Comment 18

[(not reading, please use [e-mail] instead)]

Comment on attachment 124564 [details] [diff] [review]
remove bogus comment

rs=sspitzer on the comment removal.

Comment 19

[Kathleen :Brade]

resolving as invalid; the current code seems to work as intended