New issue
Advanced search Search tips

Issue 780826 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

FileChooserParams.getAcceptTypes() uses semicolon instead of comma

Reported by m...@rcel.cz, Nov 2 2017

Issue description

Steps to reproduce the problem:
1. Use following input on a web page:

<input type="file" accept="image/*,video/*">

2. Override the onShowFileChooser() method in WebChromeClient
3. Call the getAcceptTypes() function on the received WebChromeClient.FileChooserParams

(I can provide Android project if necessary)

What is the expected behavior?
The returned array should look like this: { "image/*", "video/*" }

What went wrong?
The String array returned by getAcceptTypes() always contains only 1 element. In this case it's "image/*,video/*". 

I'm passing that to Intent.ACTION_GET_CONTENT in the EXTRA_MIME_TYPES field and because of this issue, it doesn't work as expected.

Did this work before? No 

Does this work in other browsers? Yes

Chrome version: 61.0.3163.98  Channel: stable
OS Version: 8.1.0 (OPP5.170921.005)
Flash Version: 

Html spec says:

"... the (accept) attribute must consist of a set of comma-separated tokens..."

https://html.spec.whatwg.org/multipage/input.html#file-upload-state-(type=file)

However in the source (not sure what version is currently used, chromium vs legacy webview) it seems to be split by semicolon:

    @Override
    public String[] getAcceptTypes() {
        if (mParams.acceptTypes == null)
            return new String[0];
        return mParams.acceptTypes.split(";");
    }
 
Components: Mobile>WebView
Issue/fix would be on the WebView side

Comment 2 by torne@chromium.org, Nov 2 2017

Owner: torne@chromium.org
Status: Assigned (was: Unconfirmed)
Wow, yeah, we really got that wrong :) It's not even anything to do with the web standard here: the parsing already happened on the blink side and it got split apart into separate tokens already, but we merge them back together (with commas) to pass them across the JNI boundary here: https://cs.chromium.org/chromium/src/android_webview/browser/aw_web_contents_delegate.cc?rcl=14b8c7a5fe993b3bc6de6d4db341182a809c5ed5&l=115
and then immediately attempt to split them apart again with semicolons, which clearly isn't going to work.

Will fix this.

Comment 3 by torne@chromium.org, Dec 22 2017

Hi there, sorry this got lost; I have the trivial fix uploaded to https://chromium-review.googlesource.com/c/chromium/src/+/842926 but it'd be good if you can provide your test project so I can verify it, as I've found that we don't have any automated tests for the file chooser at all and our test shell doesn't implement it either :(

Comment 4 by torne@chromium.org, Jan 2 2018

Filed issue 798422 to cover adding test code for this.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec582893ba481df66dd3f3c466ac087fbc383460

commit ec582893ba481df66dd3f3c466ac087fbc383460
Author: Torne (Richard Coles) <torne@google.com>
Date: Tue Jan 02 15:41:54 2018

Fix WebView file chooser for multiple accept-types.

We use commas to pass multiple types across the JNI boundary as a single
string (in aw_web_contents_delegate.cc), but then try to split using
semicolons on the Java side, so file upload pickers only worked
correctly for the case of a single acceptable file type. Use comma
instead when splitting.

Bug:  780826 
Change-Id: I4e360e81e71f9d565005fd026b889a74240b3795
Reviewed-on: https://chromium-review.googlesource.com/842926
Reviewed-by: Tobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526483}
[modify] https://crrev.com/ec582893ba481df66dd3f3c466ac087fbc383460/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

Comment 6 by torne@chromium.org, Jan 2 2018

Status: Fixed (was: Assigned)
Components: Blink>Storage>FileAPI
Components: -Blink>FileAPI

Sign in to add a comment