Opened 10 years ago

Closed 9 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 9 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 9 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)

Changed 10 years ago by idefix

Attachment: DriveSetupError.png added

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

Changed 10 years ago by idefix

Attachment: drivesetup.patch added

Patch to fix DriveSetups handling of illegal volume names

comment:1 Changed 10 years ago by diver

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

Initialization of the partition__"test" failed.

comment:2 Changed 10 years ago by idefix

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

Changed 10 years ago by idefix

Attachment: drivesetupspace.patch added

Patch to fix double space in error dialog of DriveSetup

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

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. :)

Changed 10 years ago by diver

Attachment: init_new_disk_nameless.png added

unnamed partition

Changed 10 years ago by diver

comment:4 Changed 10 years ago by stippi

Status: newassigned

Thanks for the patches, will apply them.

comment:5 Changed 10 years ago by diver

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

comment:6 Changed 10 years ago by stippi

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 Changed 10 years ago by idefix

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

comment:8 Changed 10 years ago by idefix

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 Changed 10 years ago by stippi

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

comment:10 Changed 10 years ago by idefix

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

Changed 10 years ago by idefix

Attachment: bfsaddon.patch added

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

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

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.

Changed 10 years ago by idefix

Patch to prevent DriveSetup create volumes without a name

comment:12 Changed 10 years ago by stippi

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 Changed 10 years ago by idefix

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 Changed 10 years ago by stippi

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 Changed 10 years ago by idefix

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

comment:16 Changed 10 years ago by stippi

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 Changed 10 years ago by idefix

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

comment:18 Changed 9 years ago by mmadia

Resolution: fixed
Status: newclosed

Thanks for the feedback idefix!

comment:19 Changed 9 years ago by idefix

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 Changed 9 years ago by idefix

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!

Changed 9 years ago by idefix

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

comment:21 Changed 9 years ago by axeld

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

Changed 9 years ago by idefix

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 Changed 9 years ago by axeld

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

comment:23 Changed 9 years ago by axeld

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

comment:24 Changed 9 years ago by idefix

Thanks!

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

comment:25 Changed 9 years ago by axeld

Resolution: fixed
Status: in-progressclosed

Thanks for the fast response!

Note: See TracTickets for help on using tickets.