Opened 10 years ago

Closed 10 years ago

#4123 closed bug (fixed)

[patch] DriveSetup doesn't use validated name when initialising volume

Reported by: idefix Owned by: axeld
Priority: normal Milestone: R1
Component: Applications/DriveSetup Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

When you have a illegal volume name like installer/test, it gets validated to installer-test.
But when DriveSetup initialises the volume, it doesn't use the validated name, but the pre-validated one: DriveSetup window showing error when trying to initialise volume with illegal volume name

Attached patch fixes this behaviour.

Attachments (9)

DriveSetupError.png (31.2 KB ) - added by idefix 10 years ago.
DriveSetup window showing error when trying to initialise volume with illegal volume name
drivesetup.patch (647 bytes ) - added by idefix 10 years ago.
Patch to fix DriveSetups handling of illegal volume names
drivesetupspace.patch (531 bytes ) - added by idefix 10 years ago.
Patch to fix double space in error dialog of DriveSetup
init_new_disk_nameless.png (70.1 KB ) - added by diver 10 years ago.
unnamed partition
init_new_disk_nameless_success.png (69.7 KB ) - added by diver 10 years ago.
bfsaddon.patch (553 bytes ) - added by idefix 10 years ago.
Path to allow DriveSetup initialise a BFS volume with a volume name that contains spaces
drivesetupunnamedvolume.patch (543 bytes ) - added by idefix 10 years ago.
Patch to prevent DriveSetup create volumes without a name
drivesetupdisablebutton.patch (2.9 KB ) - added by idefix 10 years ago.
Patch to prevent DriveSetup initialise volumes without a name by disabling the Initialize button
drivesetupdisablebutton-v2.patch (2.9 KB ) - added by idefix 10 years ago.
Updated patch to prevent DriveSetup initialise volumes without a name by disabling the Initialize button (updated to account for the recent changes in DriveSetup)

Download all attachments as: .zip

Change History (34)

by idefix, 10 years ago

Attachment: DriveSetupError.png added

DriveSetup window showing error when trying to initialise volume with illegal volume name

by idefix, 10 years ago

Attachment: drivesetup.patch added

Patch to fix DriveSetups handling of illegal volume names

comment:1 by diver, 10 years ago

While you at it there is an extra space in the alert string:

Initialization of the partition__"test" failed.

comment:2 by idefix, 10 years ago

I just noticed a double space in the error dialog text, which this patch will fix.

by idefix, 10 years ago

Attachment: drivesetupspace.patch added

Patch to fix double space in error dialog of DriveSetup

in reply to:  1 comment:3 by idefix, 10 years ago

Replying to diver:

While you at it there is an extra space in the alert string:

Initialization of the partition__"test" failed.

Yeah, I noticed that too. :)

by diver, 10 years ago

Attachment: init_new_disk_nameless.png added

unnamed partition

comment:4 by stippi, 10 years ago

Status: newassigned

Thanks for the patches, will apply them.

comment:5 by diver, 10 years ago

Stippi, should I open another bug described in the last two screenshots?

comment:6 by stippi, 10 years ago

No, it's ok to keep it in this ticket I guess. Do you mean the problem is that DriveSetup should force you to enter a name? (I think it should.)

comment:7 by idefix, 10 years ago

Or it could assign a default name like Unnamed; that's how mkfs does it.

comment:8 by idefix, 10 years ago

BTW: I have another patch coming to fix DriveSetups handling of a volume name with a space in it. I was thinking of opening another ticket for this patch, but I could also attach it to this one if you keep it open for the bug described by diver.

comment:9 by stippi, 10 years ago

Actually, Bryce could maybe also handle this, since he is working on DriveSetup anyway.

comment:10 by idefix, 10 years ago

Well, this patch will fix the volume-name-contains-spaces-bug.

by idefix, 10 years ago

Attachment: bfsaddon.patch added

Path to allow DriveSetup initialise a BFS volume with a volume name that contains spaces

in reply to:  7 comment:11 by idefix, 10 years ago

Replying to idefix:

Or it could assign a default name like Unnamed; that's how mkfs does it.

This patch makes DriveSetup do it the same way.

by idefix, 10 years ago

Patch to prevent DriveSetup create volumes without a name

comment:12 by stippi, 10 years ago

To be honest, I think an alert requesting the user to enter a name may be a better solution. I think Bryce is even working on this already, but he is not on the Trac notification list, IIRC.

comment:13 by idefix, 10 years ago

If you go that way, I would suggest to disable the Initialize button when the Name field is empty (see first screenshot).
The user is already shown quite a lot of alerts when initialising a volume.

comment:14 by stippi, 10 years ago

Ah yes, I think that's indeed the conclusion that Ingo and Bryce arrived at as well, IIRC. Sounds good to me. Still, I believe Bryce is already working on that one.

comment:15 by idefix, 10 years ago

Well, we'll see what he comes up with then... :)

comment:16 by stippi, 10 years ago

Owner: changed from stippi to bebop
Status: assignednew

Is this still valid? IIRC, a fix was part of one of the patches from Bryce.

comment:17 by idefix, 10 years ago

I just tried to initialise a volume with an empty name using a R1Alpha1 candidate image (hrev32917) and it succeeded.

comment:18 by mmadia, 10 years ago

Resolution: fixed
Status: newclosed

Thanks for the feedback idefix!

comment:19 by idefix, 10 years ago

Resolution: fixed
Status: closedreopened

I've probably worded comment:17 not clear enough, but I shouldn't have succeeded in initialising a volume with an empty name. So the issue that diver mentioned in comment:5 isn't fixed yet (well, as of hrev32917; haven't tried it with a recent version).

I will try to create a patch for this in the coming weeks if Bryce has't done so already.

comment:20 by idefix, 10 years ago

With drivesetupdisablebutton.patch, DriveSetup will disable the Initialize button when the Name field is empty. It works correctly, but I'm not entirely happy with the implementation I've chosen (it isn't immediately clear where the MSG_NAME_CHANGED message comes from.)

Suggestions for improvement are more than welcome!

by idefix, 10 years ago

Patch to prevent DriveSetup initialise volumes without a name by disabling the Initialize button

comment:21 by axeld, 10 years ago

Owner: changed from bebop to stippi
Status: reopenedassigned
Version: R1/pre-alpha1R1/Development

by idefix, 10 years ago

Updated patch to prevent DriveSetup initialise volumes without a name by disabling the Initialize button (updated to account for the recent changes in DriveSetup)

comment:22 by axeld, 10 years ago

Owner: changed from stippi to axeld
Status: assignedin-progress

comment:23 by axeld, 10 years ago

Applied the patch in hrev35524, and added some minor TODOs. Can this ticket be fixed now, or are there other show stoppers?

comment:24 by idefix, 10 years ago

Thanks!

This was the last thing to be fixed, so this ticket can be closed now.

comment:25 by axeld, 10 years ago

Resolution: fixed
Status: in-progressclosed

Thanks for the fast response!

Note: See TracTickets for help on using tickets.