Bug 69598

Summary: Pass Parsed Accept Attribute MIME Types to WebKit Clients
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebCore Misc.Assignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, dglazkov, gustavo, joepeck, jonlee, rcombs, tkent, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 70095    
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Part 1 - Deprecate String version of acceptTypes
gyuyoung.kim: commit-queue-
[PATCH] Part 2 - Switch a WebKit2 mirror struct to just use WebCore::FileChooserSettings
andersca: review+
[PATCH] Part 3 - Add acceptMIMETypes parsed list to FileChooserSettings
ddkilzer: review+
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix EFL)
gustavo: commit-queue-
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix Builds) tkent: review+

Description Joseph Pecoraro 2011-10-06 19:52:41 PDT
Rather then have each of the WebKit clients parse the MIME types from the
accept attribute string via FileChooser, WebCore should parse the string
and pass a list. WebCore already has the HTML5 compatible parser logic.

NOTE: Chromium is the only client that uses the "acceptType" attribute of
FileChooser right now. They pass it along as a string and parse it later
in chromium code.

See also:
<http://webkit.org/b/51045> [Qt] Implement HTML File Input "accept" attribute
<http://webkit.org/b/45079> HTML <input type="file"> accept attribute
Comment 1 Joseph Pecoraro 2011-10-06 20:02:25 PDT
Created attachment 110082 [details]
[PATCH] Proposed Fix

I think this is the preferable approach. Rather then passing the "accept" attribute string
to WebKit clients, we instead pass the parsed list of trimmed MIME types. This way
ports don't have to write their own, possibly differing parsing implementations and
we can make use of the HTML5 parsers already in WebCore.

I'm not very familiar with WK2, but this looks like it would affect the WebKit2 API.
Does that require any special considerations?
Comment 2 Joseph Pecoraro 2011-10-07 12:44:13 PDT
Comment on attachment 110082 [details]
[PATCH] Proposed Fix

Clearing flags. Based on some feedback I'm going to do some extra clean-up and break this up into parts.
Comment 3 Joseph Pecoraro 2011-10-07 12:45:44 PDT
Created attachment 110198 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes

Chromium folks, check out Part 3 when it is up. Instead of parsing the mime types
in chromium WebKit ports should share the implementation in WebCore.
Comment 4 Joseph Pecoraro 2011-10-07 12:46:42 PDT
Created attachment 110199 [details]
[PATCH] Part 2 - Switch a WebKit2 mirror struct to just use WebCore::FileChooserSettings
Comment 5 Joseph Pecoraro 2011-10-07 12:47:23 PDT
Created attachment 110200 [details]
[PATCH] Part 3 - Add acceptMIMETypes parsed list to FileChooserSettings
Comment 6 Gyuyoung Kim 2011-10-07 13:59:34 PDT
Comment on attachment 110198 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes

Attachment 110198 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10006296
Comment 7 Joseph Pecoraro 2011-10-07 14:30:39 PDT
(In reply to comment #6)
> (From update of attachment 110198 [details])
> Attachment 110198 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/10006296

Looks like the EFL port also used this and exposed it to their api as a char*:

    Eina_Bool (*run_open_panel)(Ewk_View_Smart_Data *sd, Evas_Object *frame, Eina_Bool allows_multiple_files, const char *accept_types, Eina_List **selected_filenames);

If they want to move away from the deprecated type It looks like they could upgrade
their API to use an Eina_List* for the MIME types list. I'll update Part 1.
Comment 8 Joseph Pecoraro 2011-10-07 14:34:49 PDT
Created attachment 110217 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix EFL)
Comment 9 Gustavo Noronha (kov) 2011-10-07 18:24:00 PDT
Comment on attachment 110217 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix EFL)

Attachment 110217 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10006349
Comment 10 Kent Tamura 2011-10-10 20:57:20 PDT
Part 1 and Part3 look reasonable.  Please fix build failures of GTK and Windows.
Comment 11 Joseph Pecoraro 2011-10-11 10:29:20 PDT
Created attachment 110533 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix Builds)

I forgot to change the WebKit2 one that gets removed in the next patch. I retroactively
created the patches. Lets give this one a shot on the bots, while my system is having
a little bit of trouble building in general.
Comment 12 Kent Tamura 2011-10-11 16:25:04 PDT
Comment on attachment 110533 [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix Builds)

Looks good.
Comment 13 David Kilzer (:ddkilzer) 2011-10-12 12:39:42 PDT
Comment on attachment 110200 [details]
[PATCH] Part 3 - Add acceptMIMETypes parsed list to FileChooserSettings

r=me
Comment 15 Joseph Pecoraro 2011-10-12 23:42:14 PDT
The bots look good.

I think this is the first time I had 3 patches reviewed by 3 different people. Thanks for coming together!

To remove the deprecated string I filed bugs on EFL and Chromium:
<http://webkit.org/b/70002> [EFL]: Move from FileChooserSettings deprecatedAcceptType to acceptMIMETypes
<http://webkit.org/b/70003> [Chromium]: Move from FileChooserSettings deprecatedAcceptType to acceptMIMETypes
Comment 16 Jon Lee 2011-11-07 11:24:22 PST
*** Bug 45079 has been marked as a duplicate of this bug. ***