Bug 184517 – input.webkitEntries does not work as expected when folder contains accented chars

• Component: DOM
• Status: RESOLVED FIXED

[Chris Dumez]

input.webkitEntries returns invalid when folder contains accented chars.

[Chris Dumez]

<rdar://problem/39265537>

[Chris Dumez]

Created attachment 337743 [details]
Patch

[Chris Dumez]

Created attachment 337745 [details]
Patch

[Chris Dumez]

Looks like some bots have trouble applying the patch due to the non-ascii character in one of the paths (which I need for testing purposes) :/

[Chris Dumez]

Created attachment 337748 [details]
Patch

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> Looks like some bots have trouble applying the patch due to the non-ascii
> character in one of the paths (which I need for testing purposes) :/

Trying again with:
$ git config --global core.quotePath false
$ git config --global core.precomposeunicode true

[EWS Watchlist]

Comment on attachment 337748 [details]
Patch

Attachment 337748 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7288624

New failing tests:
fast/forms/file/entries-api/webkitEntries-nonascii-folder-name.html

[EWS Watchlist]

Created attachment 337758 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 337748 [details]
Patch

Attachment 337748 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7288686

New failing tests:
fast/forms/file/entries-api/webkitEntries-nonascii-folder-name.html

[EWS Watchlist]

Created attachment 337760 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 337748 [details]
Patch

Attachment 337748 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7288885

New failing tests:
fast/forms/file/entries-api/webkitEntries-nonascii-folder-name.html

[EWS Watchlist]

Created attachment 337761 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Chris Dumez]

Created attachment 337763 [details]
Patch

[Chris Dumez]

Created attachment 337765 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 337765 [details]
Patch

Clearing flags on attachment: 337765

Committed r230554: <https://trac.webkit.org/changeset/230554>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jonathan Bedard]

Do we have any other examples of non-ascii folder names in WebKit?

I have a pure-svn checkout, and it doesn't seem to play nice with a non-ascii folder.

[Alexey Proskuryakov]

We don't have any files with non-ASCII names. I think that I tried to add some test years ago, and that caused problems.

Chris, pease switch to a cgi script with mod_rewrite instead. Here are some example to start: https://stackoverflow.com/questions/5218213/create-a-catch-all-handler-in-php

[Ryan Haddad]

Reverted r230554 for reason:

The non-ASCII character in the filename causes issues with SVN

Committed r230586: <https://trac.webkit.org/changeset/230586>

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #18)
> We don't have any files with non-ASCII names. I think that I tried to add
> some test years ago, and that caused problems.
> 
> Chris, pease switch to a cgi script with mod_rewrite instead. Here are some
> example to start:
> https://stackoverflow.com/questions/5218213/create-a-catch-all-handler-in-php

This is not a viable option, this is using the filesystem. The file is not and cannot be served by a server.

[Chris Dumez]

(In reply to Chris Dumez from comment #20)
> (In reply to Alexey Proskuryakov from comment #18)
> > We don't have any files with non-ASCII names. I think that I tried to add
> > some test years ago, and that caused problems.
> > 
> > Chris, pease switch to a cgi script with mod_rewrite instead. Here are some
> > example to start:
> > https://stackoverflow.com/questions/5218213/create-a-catch-all-handler-in-php
> 
> This is not a viable option, this is using the filesystem. The file is not
> and cannot be served by a server.

I'll check if I can write an API test instead.

[Chris Dumez]

Created attachment 337822 [details]
Patch

[Alexey Proskuryakov]

> This is not a viable option, this is using the filesystem. The file is not and cannot be served by a server.

Indeed. But does the path need to actually exit? If not, beginDragWithFiles sounds like it may work here. This is what we are using to simulate adding various fake files to form inputs. It's WebKit1 only, so long term we need a way to simulate arbitrary file input values in WebKit2.

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #23)
> > This is not a viable option, this is using the filesystem. The file is not and cannot be served by a server.
> 
> Indeed. But does the path need to actually exit? If not, beginDragWithFiles
> sounds like it may work here. This is what we are using to simulate adding
> various fake files to form inputs. It's WebKit1 only, so long term we need a
> way to simulate arbitrary file input values in WebKit2.

Since the path is not exposed the JS, the test relied on being able (or not) to read the file contents from disk. Since when the path is wrongly encoded, we fail to read the file.

Anyway, my latest patch iteration uses an API test instead.

[Alex Christensen]

Comment on attachment 337822 [details]
Patch

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

> Source/WebCore/platform/posix/FileSystemPOSIX.cpp:354
> +    return String::fromUTF8(dirname(fsRep.mutableData()));

Maybe we should just call data instead of mutableData now.

[Chris Dumez]

Comment on attachment 337822 [details]
Patch

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

>> Source/WebCore/platform/posix/FileSystemPOSIX.cpp:354
>> +    return String::fromUTF8(dirname(fsRep.mutableData()));
> 
> Maybe we should just call data instead of mutableData now.

dirname() takes in a char*, not a const char*

[WebKit Commit Bot]

Comment on attachment 337822 [details]
Patch

Clearing flags on attachment: 337822

Committed r230639: <https://trac.webkit.org/changeset/230639>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.