Bug 69598 - Pass Parsed Accept Attribute MIME Types to WebKit Clients
: Pass Parsed Accept Attribute MIME Types to WebKit Clients
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 70095
  Show dependency treegraph
 
Reported: 2011-10-06 19:52 PST by
Modified: 2011-11-07 11:24 PST (History)


Attachments
[PATCH] Proposed Fix (7.13 KB, patch)
2011-10-06 20:02 PST, Joseph Pecoraro
no flags Review Patch | Details | Formatted Diff | Diff
[PATCH] Part 1 - Deprecate String version of acceptTypes (3.70 KB, patch)
2011-10-07 12:45 PST, Joseph Pecoraro
gyuyoung.kim: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
[PATCH] Part 2 - Switch a WebKit2 mirror struct to just use WebCore::FileChooserSettings (16.35 KB, patch)
2011-10-07 12:46 PST, Joseph Pecoraro
andersca: review+
Review Patch | Details | Formatted Diff | Diff
[PATCH] Part 3 - Add acceptMIMETypes parsed list to FileChooserSettings (5.75 KB, patch)
2011-10-07 12:47 PST, Joseph Pecoraro
ddkilzer: review+
Review Patch | Details | Formatted Diff | Diff
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix EFL) (6.20 KB, patch)
2011-10-07 14:34 PST, Joseph Pecoraro
gns: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix Builds) (8.03 KB, patch)
2011-10-11 10:29 PST, Joseph Pecoraro
tkent: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-10-06 19:52:41 PST
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 From 2011-10-06 20:02:25 PST -------
Created an attachment (id=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 From 2011-10-07 12:44:13 PST -------
(From update of attachment 110082 [details])
Clearing flags. Based on some feedback I'm going to do some extra clean-up and break this up into parts.
------- Comment #3 From 2011-10-07 12:45:44 PST -------
Created an attachment (id=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 From 2011-10-07 12:46:42 PST -------
Created an attachment (id=110199) [details]
[PATCH] Part 2 - Switch a WebKit2 mirror struct to just use WebCore::FileChooserSettings
------- Comment #5 From 2011-10-07 12:47:23 PST -------
Created an attachment (id=110200) [details]
[PATCH] Part 3 - Add acceptMIMETypes parsed list to FileChooserSettings
------- Comment #6 From 2011-10-07 13:59:34 PST -------
(From update of attachment 110198 [details])
Attachment 110198 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10006296
------- Comment #7 From 2011-10-07 14:30:39 PST -------
(In reply to comment #6)
> (From update of attachment 110198 [details] [details])
> Attachment 110198 [details] [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 From 2011-10-07 14:34:49 PST -------
Created an attachment (id=110217) [details]
[PATCH] Part 1 - Deprecate String version of acceptTypes (Fix EFL)
------- Comment #9 From 2011-10-07 18:24:00 PST -------
(From update of attachment 110217 [details])
Attachment 110217 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10006349
------- Comment #10 From 2011-10-10 20:57:20 PST -------
Part 1 and Part3 look reasonable.  Please fix build failures of GTK and Windows.
------- Comment #11 From 2011-10-11 10:29:20 PST -------
Created an attachment (id=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 From 2011-10-11 16:25:04 PST -------
(From update of attachment 110533 [details])
Looks good.
------- Comment #13 From 2011-10-12 12:39:42 PST -------
(From update of attachment 110200 [details])
r=me
------- Comment #15 From 2011-10-12 23:42:14 PST -------
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 From 2011-11-07 11:24:22 PST -------
*** Bug 45079 has been marked as a duplicate of this bug. ***