Bug 194012 - Fix WebKitTestRunner's testPath with Windows full paths
Summary: Fix WebKitTestRunner's testPath with Windows full paths
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-30 01:20 PST by Fujii Hironori
Modified: 2019-01-31 02:28 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.88 KB, patch)
2019-01-30 01:29 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (1.80 KB, patch)
2019-01-31 02:24 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2019-01-30 01:20:25 PST
[Win][WebKitTestRunner] Can't get test header options in webkit-test-runner magic comments

Some test cases are failing due to failing to get test options.
For example, legacy-animation-engine/animations/generic-from-to.html
It has the following magic comment:

> <!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=false ] -->
Comment 1 Fujii Hironori 2019-01-30 01:29:13 PST
Created attachment 360564 [details]
Patch
Comment 2 Don Olmstead 2019-01-30 09:24:06 PST
From https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/

> For the local Windows file path
> C:\Documents and Settings\davris\FileSchemeURIs.doc
>
> The corresponding valid file URI in Windows is:
> file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc

So three forward slashes seem fine its just we're not handling it correctly.

Isn't there anything that converts a File URI into a path rather than having special parsing here?
Comment 3 Alex Christensen 2019-01-30 09:33:06 PST
Comment on attachment 360564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360564&action=review

> Tools/WebKitTestRunner/TestController.cpp:1095
> +        // Remove the first '/' if it starts with like "/C:/".

starts with something like
Comment 4 Alex Christensen 2019-01-30 09:34:26 PST
Comment on attachment 360564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360564&action=review

> Tools/ChangeLog:3
> +        [Win][WebKitTestRunner] Can't get test header options in webkit-test-runner magic comments

I think this needs a better title.  Something like "Fix WebKitTestRunner's testPath with Windows full paths"
Comment 5 Alex Christensen 2019-01-30 10:41:38 PST
(In reply to Don Olmstead from comment #2)
> From https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/
> 
> > For the local Windows file path
> > C:\Documents and Settings\davris\FileSchemeURIs.doc
> >
> > The corresponding valid file URI in Windows is:
> > file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc
> 
> So three forward slashes seem fine its just we're not handling it correctly.
> 
> Isn't there anything that converts a File URI into a path rather than having
> special parsing here?

Certainly not in the URL class.  This is how URLs behave:

<script>
alert(new URL("file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc").pathname);
</script>

It's strange, but all browsers agree and it's in the specification.
Comment 6 Fujii Hironori 2019-01-30 17:12:55 PST
(In reply to Don Olmstead from comment #2)
> From https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/

Interesting. Should I use PathCreateFromUrl and UrlCreateFromPath API?
Comment 7 Fujii Hironori 2019-01-31 02:24:44 PST
Created attachment 360712 [details]
Patch
Comment 8 Fujii Hironori 2019-01-31 02:27:45 PST
Committed r240775: <https://trac.webkit.org/changeset/240775>
Comment 9 Radar WebKit Bug Importer 2019-01-31 02:28:28 PST
<rdar://problem/47697394>