Bug 75438

Created attachment 120882 [details] Patch

Eric, can you please review? Without this patch, I get the following error on my Lion box: 2012-01-02 17:08:14.094 osascript[95311:f07] CFURLCreateWithString was passed this invalid URL string: '/Volumes/Data/WebKit/Source/WebKit/chromium/webkit/Debug/layout-test-results/results.html' (a file system path instead of an URL string). The URL created will not work with most file URL functions. CFURLCreateWithFileSystemPath or CFURLCreateWithFileSystemPathRelativeToBase should be used instead.

Comment on attachment 120882 [details] Patch It would be better to add this in a mockable fashion. like adding a file_url_for_path to FileSystem or just making open_url do this automatically? I'm surprised this only affects chromium?

I believe I've run the tests on Lion before and had it open the results correctly. So I'm surprised that this is a new issue?

I've had the same problem. It seems to affect the Chromium port, not the Apple port.

Comment on attachment 120882 [details] Patch Test?

Some ports override the show_results_html method. I was a bit hesitant to add this to open_url, because there might be other call sites that pass in correct URLs. I'd then first have to try to guess whether it's a filesystem path or already a valid URL. Adding file_path_as_url to FileSystem sounds like a good idea. I'll do that and write a test for it.

Created attachment 120936 [details] Patch

Comment on attachment 120936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120936&action=review > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:66 > + self.assertEqual(fs.file_path_as_url('/foo'), > + 'file:///foo') Is this going to pass on Windows?

(In reply to comment #9) > (From update of attachment 120936 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=120936&action=review > > > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:66 > > + self.assertEqual(fs.file_path_as_url('/foo'), > > + 'file:///foo') > > Is this going to pass on Windows? I don't know. I don't have a windows checkout of webkit handy to try and no bots execute those tests :-/

(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 120936 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=120936&action=review > > > > > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:66 > > > + self.assertEqual(fs.file_path_as_url('/foo'), > > > + 'file:///foo') > > > > Is this going to pass on Windows? > > I don't know. I don't have a windows checkout of webkit handy to try and no bots execute those tests :-/ All the bots execute tehse tests. :) I don't think the EWS bots do today... but they used to.

(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > (From update of attachment 120936 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=120936&action=review > > > > > > > Tools/Scripts/webkitpy/common/system/filesystem_unittest.py:66 > > > > + self.assertEqual(fs.file_path_as_url('/foo'), > > > > + 'file:///foo') > > > > > > Is this going to pass on Windows? > > > > I don't know. I don't have a windows checkout of webkit handy to try and no bots execute those tests :-/ > > All the bots execute tehse tests. :) I don't think the EWS bots do today... but they used to. ah, ok, I was mistaken, just the slow scm tests aren't executed. I'll watch the buildbots for the result then

Comment on attachment 120936 [details] Patch Clearing flags on attachment: 120936 Committed r103984: <http://trac.webkit.org/changeset/103984>

All reviewed patches have been landed. Closing bug.

Hm. We already had a path module in webkitpy.common.system that provided an abspath_to_uri() method that did the same thing. It's not clear to me that this should really live on filesystem instead (although at least that would give you predictable behavior using a mock filesystem on windows) but we probably shouldn't have both.

(In reply to comment #15) > Hm. We already had a path module in webkitpy.common.system that provided an abspath_to_uri() method that did the same thing. It's not clear to me that this should really live on filesystem instead (although at least that would give you predictable behavior using a mock filesystem on windows) but we probably shouldn't have both. Oh, I didn't see that I'll try to come up with a way to unify this