RESOLVED FIXED 72661
Switch to a more modern approach to retrieving the startup volume name
https://bugs.webkit.org/show_bug.cgi?id=72661
Summary Switch to a more modern approach to retrieving the startup volume name
Mark Rowe (bdash)
Reported 2011-11-17 14:37:38 PST
As it says.
Attachments
Patch v1 (3.58 KB, patch)
2011-11-17 14:40 PST, Mark Rowe (bdash)
mitz: review+
Patch v2 (9.28 KB, patch)
2011-11-20 16:17 PST, Mark Rowe (bdash)
mitz: review+
webkit.review.bot: commit-queue-
Mark Rowe (bdash)
Comment 1 2011-11-17 14:40:13 PST
Created attachment 115686 [details] Patch v1 Patch!
Alexey Proskuryakov
Comment 2 2011-11-18 11:25:37 PST
This method is used to manipulate a path, and it seems very fragile to expect that display name matches path component. If nothing else, path components use different Unicode normalization than regular strings in OS X.
Darin Adler
Comment 3 2011-11-18 11:57:00 PST
Comment on attachment 115686 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=115686&action=review > Source/WebKit/mac/Misc/WebNSFileManagerExtras.mm:95 > - (NSString *)_webkit_startupVolumeName > { > - NSString *path = [self _webkit_carbonPathForPath:@"/"]; > - return [path substringToIndex:[path length]-1]; > + return [[[NSFileManager defaultManager] componentsToDisplayForPath:@"/"] objectAtIndex:0]; > } While it may work OK, this doesn’t seem quite right. This method is not used to display the volume name. It’s used to compare the name with the first component in a string extracted using -[NSString pathComponents], which I do not believe does any “display” processing. So if “components to display” has any meaning other than “components” for volumes then the code will not work well.
Darin Adler
Comment 4 2011-11-18 11:57:23 PST
(In reply to comment #2) > This method is used to manipulate a path, and it seems very fragile to expect that display name matches path component. If nothing else, path components use different Unicode normalization than regular strings in OS X. Ah, it seems that Alexey and I are saying the same thing.
Mark Rowe (bdash)
Comment 5 2011-11-20 16:17:31 PST
Created attachment 116006 [details] Patch v2 This patch uses DiskArbitration to find the volume name of the startup volume.
Mark Rowe (bdash)
Comment 6 2011-11-20 16:18:47 PST
This is the same way of retrieving the name that DiskArbitration uses internally when setting up the /Volumes/Blargh mount point directories and symlinks.
WebKit Review Bot
Comment 7 2011-11-20 16:44:55 PST
Comment on attachment 116006 [details] Patch v2 Attachment 116006 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10533091 New failing tests: platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html
Mark Rowe (bdash)
Comment 8 2011-11-21 13:44:44 PST
Landed in r100954.
Note You need to log in before you can comment on or make changes to this bug.