JDK-8089915 : Input of type file doesn't honor "accept" attribute.
Type:Bug
Component:javafx
Sub-Component:web
Affected Version:8,9
Priority:P4
Status:Resolved
Resolution:Fixed
Submitted:2013-06-04
Updated:2017-01-30
Resolved:2016-11-18
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
Input of type file doesn't honor "accept" attribute.
You can use attached file to reproduce the issue.
Comments
changeset 10139:e360b58bfb4d tip
8089915: Input of type file doesn't honor "accept" attribute.
Summary: Implemented HTML5 input tag accept attribute.
Reviewed-by: kcr, mbilla, arajkumar, ghb
http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/e360b58bfb4d
18-11-2016
Regarding srt (SubRip text file format) extension
An SRT file is a subtitle file saved in the SubRip file format.
It is supported by several video formats such as DivX and DVD and used by various video playback programs.
SRT files contain subtitle information, which includes the sequential number of subtitles, start and end timecode, subtitle text.
17-11-2016
To be clear, I'm OK with the .10 version as-is. We can look at .xml (and .log, etc) in a follow-on JBS issue.
16-11-2016
+1
As for the xml addition, I'm sure there are others that could be considered text files as well (e.g., *.log), so it might be worth a follow-up bug. Also, I've never heard of ".srt" as an extension....do we use it anywhere else in our system? To be clear, I don't want to hold up getting this in, so I think any futher changes could be handled in a follow-up "cleanup" bug.
16-11-2016
Do you want add xml type also for "Text Files" as * ".xml" ?
FileExtensionInfo.add("text", "Text Files", "*.txt", "*.csv", "*.text", "*.ttf", "*.sdf", "*.srt", "*.htm", "*.html");
+1 (with xml MimeType addition)
The following seems to work:
// Remove old filters, add specific filters, then add generic filter
chooser.getExtensionFilters().clear();
if (mimeFilters != null && !mimeFilters.isEmpty()) {
addMimeFilters(chooser, mimeFilters);
}
chooser.getExtensionFilters().addAll(new ExtensionFilter("All Files", "*.*"));
15-11-2016
OK, I've done some more testing and I see that Firefox on both Windows and Mac has the "Image Files" selected by default, but does have the "All files" option that a user can select. Chrome behaves the same way.
This seems like a good behavior, so one possibility is to always add the "All Files" filter (as done in the .09 version), but make the first provided filter be the default. This might be as easy as adding the "All Files" filter last (which looks like it would match Chrome's behavior).
15-11-2016
I think the bug is that you now do the following unconditionally in chooseFile:
+ chooser.getExtensionFilters().addAll(new ExtensionFilter("All Files", "*.*"));
This is only wanted for the case where no filters are defined.
So rather than:
// Remove old filters and add generic filter
chooser.getExtensionFilters().clear();
chooser.getExtensionFilters().addAll(new ExtensionFilter("All Files", "*.*"));
if (mimeFilters != null && !mimeFilters.isEmpty()) {
addMimeFilters(chooser, mimeFilters);
}
I think you want something like the following:
// Remove old filters and add generic filter
chooser.getExtensionFilters().clear();
if (mimeFilters != null && !mimeFilters.isEmpty()) {
addMimeFilters(chooser, mimeFilters);
} else {
chooser.getExtensionFilters().addAll(new ExtensionFilter("All Files", "*.*"));
}
15-11-2016
The incremental build bug is fixed.
My testing of the .09 version on Mac shows a functional issue that I didn't see with whichever previous version I was testing last time (the .05 version I think). I will test on Windows, too, but it looks like there if a single filter is specified, then two filters are used, one that accepts everything (which is used by default), and the one that accepts only the specified files. This is not expected or desired.
15-11-2016
I was unable to test this since it appears that incremental builds of WebKit are broken -- I'll file a new bug for that.
I'll test and finish my review after I am able to do a clean build.
+1,
Please rename MIMEExtensionInfo --> FileExtensionInfo or better name as you did for rev 04.
No need to post another webrev if this is the only change.
[@mbilla] : The "if" check was redundant -> removed.
Check for subType.equals("*.*") is not needed anymore, I'm checking whether mime type is in the map, if not present only default filter will be applied.
Above behaviour is same as for other browsers.
11-11-2016
1.
if (!Arrays.asList(strArray).contains(mimeStr)) {
+ // Do nothing
+ } else {
+ mimeDescription = extensionValue.description + " " + "(" + mimeStr + ")";
+ chooser.getExtensionFilters().addAll(new ExtensionFilter(mimeDescription, mimeStr));
+ }
Can you modify the above code like below?
if (Arrays.asList(strArray).contains(mimeStr)) {
+ mimeDescription = extensionValue.description + " " + "(" + mimeStr + ")";
+ chooser.getExtensionFilters().addAll(new ExtensionFilter(mimeDescription, mimeStr));
+ }
2. Again indentation is not correct at below places:
closing braces for if (mimeString.contains("/")) { is at 3 spaces instead of 4.
Similar issue for if (extensionValue != null) {
3. Why you removed the below check in .06 patch ?
} else if (subType.equals("*.*")) {
+ return true;
11-11-2016
Looks like some of review comments are not addressed yet.
private String getExtensions(String srcStr1, String destStr1, String srcStr2, String destStr2) {
+ return Arrays.asList(extension).toString().replace(srcStr1, destStr1).replace(srcStr2, destStr2);
+ }
Don't rely on toString() method of an ArrayList which can change without our control. Please define 2 methods like below in MIMEExtensionInfo to get ExtensionFilter.
private static class MIMEExtensionInfo {
private String description;
private List<String> extensions;
static void add(String type, String description, String... extensions) {
MIMEExtensionInfo info = new MIMEExtensionInfo();
info.description = description;
info.extensions = Arrays.asList(extensions);
fileExtensionMap.put(type, info);
}
private String getExtensionFilter(String type) {
String desc = this.description + " ";
if (type.equals("*")) {
desc += extensions.stream().collect(java.util.stream.Collectors.joining(", ", "(", ")"));
return new ExtensionFilter(desc, this.extensions);
} else if (extensions.contains(type)) {
desc += type;
return new ExtensionFilter(desc, type);
}
}
}
private void addSpecificFilters(FileChooser chooser, String mimeString) {
if (mimeString.contains("/")) {
final String splittedMime[] = mimeString.split("/");
final String mainType = splittedMime[0];
final String subType = splittedMime[1];
final MIMEExtensionInfo extensionValue = fileExtensionMap.get(mainType);
if (extensionValue != null) {
chooser.getExtensionFilters().addAll(extensionValue.getExtensionFilter(subType));
}
}
}
This should serve the same purpose and has better readability I guess.
11-11-2016
http://cr.openjdk.java.net/~asrivastava/8089915/webrev.06/
Patch updated.
Behaviour now matches with browsers.
Added test file also.
10-11-2016
mimeDescription = extensionValue.description + " " +
+ (Arrays.asList(extensionValue.extension)).toString().replace("[", "(").replace("]", ")");
Add method to MIMEExtensionInfo which returns the pretty string to show as part of file chooser drop down.
String mimeStr = "*." + subType;
+ String strArray[] = ((Arrays.asList(extensionValue.extension)).toString().
+ replace("[", "").replace("]", "")).split(", ");
+ if (!Arrays.asList(strArray).contains(mimeStr)) {
+ // Do nothing
+ } else {
+ mimeDescription = extensionValue.description + " " + "(" + mimeStr + ")";
+ chooser.getExtensionFilters().addAll(new ExtensionFilter(mimeDescription, mimeStr));
+ }
What is the purpose of strArray variable?
10-11-2016
The fix itself look OK to me now. I still see a few spacing / style issues.
1. The new class is declared as "static private" -- convention is "private static"
2. The closing curly brace of the static block is indented more than it should be.
3. Indentation is not aligned for calls to chooser.getExtensionFilters().addAll -- either indent 8 spaces or line up all arguments
4. Extra space in second 'return false ;' (addMimeFilters method)
+ for(String mimeType: types) {
Leave a space before and after : in for-each loop
+ String subTypes[] = mimeString.split("/");
Using array index is hard to read, better have 2 variables like mainType, subType.
String splittedMime = mimeString.split("/");
String mainType = splittedMime[0];
String subType = splittedMime[1];
+ private static Map<String, String[]> fileExtensionMap;
+ private static Map<String, String> fileExtensionStringMap;
You can remove the second map by introducing a new Type to hold filter description like below,
static Map<String, MIMEExtensionInfo> mimeMap = new HashMap<>();
static private class MIMEExtensionInfo {
String type;
String description;
String extension[];
static void create(String type, String description, String... extension) {
MIMEExtensionInfo info = new MIMEExtensionInfo();
info.type = type;
info.description = description;
info.extension = extension;
mimeMap.put(type, info);
}
}
static {
MIMEExtensionInfo.create("video", "Video Files", "*.webm", "*.mp4");
MIMEExtensionInfo.create("audio", "Audio Files", "*.mp3", "*.aac");
}
07-11-2016
Missed this earlier..
1) There is some indentation problem for function addSpecificFilters
addSpecificFilters is starting at 6 spaces at begining instead of 4 spaces and some of subsequent lines starting at 5 spaces..
if (mimeString.contains("/")) { is starting at 5 spaces and corresponding else is starting at 6 spaces..
Can you check this fucntion again and correct the indentation issue?
2) Also indentation for below 2 lines:
env->CallObjectMethod(m_webPage, chooseFileMID,
(jstring)initialFilename, multiple,
(jstring)(builder.toString().toJavaString(env)))));
3) If i give accept input as "text/*.*" , then you are appending "*." to mimeStr and
mimeStr will be *.*.*
what is the expected behaviour ? can you please check this case?
UIClientImpl
better name for mimeMap (filetypeMap or something better). Field name "mimeMap" our intent is for file type mapping for rather than mime type mapping i.e accept="file_extension|audio/*|video/*|image/*|media_type" (Ref from w3school).
Neither the HTML spec specifies the mime type nor the javafx.stage.FileChooser accepts or takes input of mimetypes to enable ExtensionFilter.
Currently 'audio', 'video', 'text' and 'image' filter is implemented, Its good have 'file_extension' and 'media_type' (This filter and extension is quite lengthy).
nit:
UIClient.java :
import javafx.stage.FileChooser; is redundant.
replace multi line comment for chooseFile
ChromeClientJava::runOpenPanel -- split arguments in multiple line
fwkChooseFile(..., String mimeFilters) if you are renaming 'mimeMap' (above comment), please rename this argument as well.
UIClientImpl.java
Its good to have public methods in the beginning and private at the last. Please retain the chooseFile first and remaining private methods later (after chooseFile() implementation).
Add 'noreg-hard' label as we don't have regression test for this fix.
04-11-2016
http://cr.openjdk.java.net/~asrivastava/8089915/webrev.02/
Incorporated Kevin comments.
04-11-2016
There's the same issue happened on Mac10.11-x64 with jdk8u121b07-64bit
04-11-2016
[@mbilla] : Whether mime type is supported or not is detected by underlying layers.Current behaviour is just for selecting files.
Eg:
video/abcd -> Wont show any file.
But, if we have a file as tt.abcd, file will be selected. Any other operation on the file will be handled by respective layers.
04-11-2016
There's the same issue happened on Win7-x64-jdk8u122b07-64bit .
04-11-2016
Please correct me, in case my understanding is wrong..
If mimetype is text/html ( html is not present in mimeMap currently) and as per below code, mimestr will be " *.html" (since u r appending "*.").
is it ok even if "html" is not present in mimeMap ? OR u want to add html also to mimeMap?
mimeMap.put("text",new String[] {"*.txt", "*.csv", "*.text", "*.ttf", "*.sdf", "*.srt"});
Also before appending "*.", do you want to check whether subTypes[1] is actually a valid type and present in mimeMap? In case if i pass an invalid type as text/abcd, not sure about the behaviour. Can you add the spec which you referred?
+ if (subTypes[1].equals("*")) {
+ chooser.getExtensionFilters().addAll(new ExtensionFilter(mimeStringMap.get(subTypes[0]), mimeMap.get(subTypes[0])));
+ } else {
+ mimeStr = "*." + subTypes[1];
+ chooser.getExtensionFilters().addAll(new ExtensionFilter(mimeStringMap.get(subTypes[0]), mimeStr));
+ }
+ } else {
+ throw new MimeTypeParseException();
+ }
03-11-2016
minor comment:
please enter space between "if" and "("
if(mimeFilters != null && !mimeFilters.isEmpty()) {
03-11-2016
Even more to the point: why use an exception in the first place? You construct and throw it within the same method and it never propagates out of that method, so isn't needed at all. Please refactor it to do what you are trying to do some other way.
03-11-2016
I didn't notice this earlier, but your patch adds a dependency on the java.activation module, which is not desirable. Additionally, since you didn't add the module in a "requires" clause in module-info.java, it will fail with a NoClassDefFoundError at runtime in our current build, and will fail at compile time once we start building with a Jigsaw-aware JDK next week.
Please use a different exception for this as there is no good reason to use the exception you chose. Please choose a RuntimeException since we do not need / want a checked exception here.
Please add a w3c spec which describes about this attribute.
+ struct WebFileChooserParams {
+ bool multiSelect;
+ bool directory;
+ Vector<String> acceptMIMETypes;
+ Vector<String> selectedFiles;
+ String acceptTypes;
+ String initialValue;
+ };
+ WebFileChooserParams params;
What is the use of new type "WebFileChooserParams"? I couldn't find a need for it.
if (subTypes[0].equals("audio")) {
+ addAudioFilters(chooser, subTypes[1]);
+ } else if (subTypes[0].equals("video")) {
+ addVideoFilters(chooser, subTypes[1]);
+ } else if (subTypes[0].equals("image")) {
+ addImageFilters(chooser, subTypes[1]);
+ } else if (subTypes[0].equals("text")) {
+ addTextFilters(chooser, subTypes[1]);
+ } else {
+ throw new MimeTypeParseException();
+ }
Sounds like you can generalize the creation of ExtensionFilter using an array and map, something like below?
Map<String, String[]> mimeMap;
mimeMap.add("video", {"webm", "ogg", "mp4"});
mimeMap.add("image", {"png", "jpeg", "git"});
...
for (mime : mimeMap) {
...
}
Some of the mime types are not supported by us(e.g. video/webm?). Should we consider those unsupported formats here?
You can find a list of supported mime types in MIMETypeRegistryJava.cpp(->Utilities.java) and MIMETypeRegistry.cpp
Do you have a plan to write test cases(manual?) for this feature?
27-10-2016
http://cr.openjdk.java.net/~asrivastava/8089915/webrev.00/
Please review.
Tested on Win-64
27-10-2016
There's the same issue happened on Mac10.12-beta--x64 with jdk8u112b15-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
26-09-2016
There's the same issue happened on Win7-x64-jdk8u112b15-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
26-09-2016
There's the same issue happened on Ubuntu16.04-x64 with jdk9b137-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
23-09-2016
There's the same issue happened on win7-x64 with jdk8u112b09-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
23-08-2016
There's the same issue happened on Mac10.12-Beta-x64 with jdk8u102b14-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
16-08-2016
There's the same issue happened on Ubuntu16.04-x64 with jdk8u112b07-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
05-08-2016
There's the same issue happened on win7-x86 with jdk8u112b07-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
05-08-2016
There's the same issue happened on mac10.11-x64 with jdk8u111b07-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
04-08-2016
There's the same issue happened on Ubuntu16.04-x86 with jdk8u112b06-32bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
01-08-2016
There's the same issue happened on win10-x64 with jdk9b127-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
25-07-2016
There's the same issue happened on mac10.10-x64 with jdk9b126-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
12-07-2016
There's the same issue happened on win10-x64 with jdk9b126-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
11-07-2016
There's the same issue happened on Ubuntu16.04-x64 with jdk9b125-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
01-07-2016
There's the same issue happened on Win7-x64 with jdk8u92b33-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
17-06-2016
There's the same issue happened on Win7-x64 with jdk8u112b01-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
16-06-2016
There's the same issue happened on Ubuntu16.04-x64 with jdk8u112b01-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
16-06-2016
There's the same issue happened on Mac10.11-x64 with jdk8u112b01-64bit
RULE "com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept" any any
15-06-2016
There's the same issue happened on win7-x64 with jdk8u102b08-64bit
18-05-2016
There's the same issue happened on Ubuntu14.04-x64-jdk8u102b06-64bit
Affected tests:
RULE com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept any any
10-05-2016
There's the same issue happened on win7-x86 with jdk8u102b06-32bit
06-05-2016
There's the same issue happened on Mac10.11-x64-jdk8u101b06-64bit
Affected tests:
RULE com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept any any
04-05-2016
There's the same issue happened on Ubuntu14.04-x86 with jdk8u102b05-32bit .
Affected tests:
RULE com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept any any
03-05-2016
There's the same issue happened on Ubuntu14.04-x64 with jdk9b113-64bit .
Affected tests:
RULE com/sun/fx/webnode/tests/html5/input/file/FileChooser.java#accept any any
14-04-2016
There's the same issue happened on Mac10.10-x64 with jdk9b113-64bit .
12-04-2016
There's the same issue happened on win7-x86 with jdk8u73b31-32bit
23-03-2016
There's the same issue happened on ubuntu16.04-x64 with jdk8u73b02-64bit.
23-03-2016
There's the same issue happened on Mac10.11-x64 with jdk8u73b31-64bit.
23-03-2016
There's the same issue happened on win7-x64 with jdk8u76b08-64bit
17-02-2016
There's the same issue happened on win7-x86 with jdk8u76b06-32bit.
05-02-2016
There's the same issue happened on Mac10.11-x64 with jdk8u75b06-64bit.