Closed Bug 973931 Opened 6 years ago Closed 6 years ago

[OS.File] makeDir should not fail if the directory is a root in the format "D:"

Categories

(Toolkit :: OS.File, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox27 --- wontfix
firefox28 + fixed
firefox29 + fixed
firefox30 + fixed
b2g-v1.3 --- fixed

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

Attachments

(1 file)

Bug 960046 fixed makeDir for the root directory case, but not when the directory is specified without a trailing backslash on Windows.

I guess, but I couldn't verify, that changing this check for "absolute" to a check for "winDrive" can fix the issue:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/osfile/modules/osfile_win_front.jsm#467
Blocks: 958899
Attached patch The patchSplinter Review
This patch fixes the issue in bug 958899. I also removed a redundant test.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #8380332 - Flags: review?(dteller)
Tracking this bug in addition to bug 958899.
Comment on attachment 8380332 [details] [diff] [review]
The patch

Review of attachment 8380332 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the comments below addressed

::: toolkit/components/osfile/modules/osfile_win_front.jsm
@@ +461,5 @@
>         // This is always the case with root directories
>         if( splitPath.components[splitPath.components.length - 1].length === 0 ) {
>           splitPath.components.pop();
>         }
>         // One component and an absolute path implies a directory root.

Please fix the comment.

::: toolkit/components/osfile/tests/xpcshell/test_makeDir.js
@@ +63,1 @@
>    }

I'd like to keep the second part of the test, to ensure that we're not breaking another important characteristic of makeDir.
Attachment #8380332 - Flags: review?(dteller) → review+
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #3)
> I'd like to keep the second part of the test, to ensure that we're not
> breaking another important characteristic of makeDir.

Isn't that already tested a few lines above?
Comment on attachment 8380332 [details] [diff] [review]
The patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 958899
User impact if declined: Inability to download to a root directory like "D:"
Testing completed (on m-c, etc.): Tested locally, just landed on inbound
Risk to taking this patch (and alternatives if risky): Minimal, has regression tests
String or IDL/UUID changes made by this patch: None
Attachment #8380332 - Flags: approval-mozilla-beta?
Attachment #8380332 - Flags: approval-mozilla-aurora?
I am waiting for the patch to land in m-c before approving it.
https://hg.mozilla.org/mozilla-central/rev/3cc8da2bf746
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Attachment #8380332 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8380332 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.