[Webkit-unassigned] [Bug 87283] [GTK] run-file-chooser signal for WebKit1

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=87283





--- Comment #5 from Mario Sanchez Prada <msanchez at igalia.com>  2012-05-25 01:32:33 PST ---
(In reply to comment #4)
> Created an attachment (id=143953)
 --> (https://bugs.webkit.org/attachment.cgi?id=143953&action=review) [details]
> Attempting to solve test failure
> 
> Tricky, I think.

Yes, it's indeed tricky :-)

> webkit_file_chooser_request_get_selected_files() calls into request->priv-
> chooser->settings().selectedFiles. However, this doesn't seem to be updated 
> when a new selection is made.

This is the expected behavior, although I agree with you that it's confusing to say the least, as I had the same doubts when working on this.

The reason why this happens is that the list of selected files in the filechooser is generated when the user initiates the logic, by pressing in a 'upload' button, and won't change during the whole lifecycle of the 'file chooser request', I think.

To make it clear, this is a list of the steps that happen in a typical workflow like this:

  1. User clicks on a 'upload' <input type='file'> button
  2. The runOpenPanel stuff is triggered from WebCore, resulting on calling to wkgtk's implementation of ChromeClient::runOpenPanel
  3. The signal 'run-file-chooser' signal is emitted from the WebView, with an instance of WebKitFileChooserRequest with the details of the event (selected files that might be already present _from previous times the user opened the dialog_, mime type filters to be applied)
  4. The application handles the signal, showing a dialog to the user so (s)he can select local files. The application might take into account, when creating that dialog the following info:
         - selected files in the request (they could pre-select them in the dialog)
         - accepted mime types (they could decide to filter other mime types out)
         - allows multiple selection (so that more than one file could be selected)
  5. The user makes a choice and closes the dialog, which would result on a call to webkit_file_chooser_request_choose_files(), or just cancels it (webkit_file_chooser_request_cancel()).
  6. The whole process finishes at this point, and nobody should care anymore about that WebKitFileChooserRequest object, that should get disposed from this point on.

So, the thing is that if you open the file chooser request for the first time, selectedFiles in WebCore will be an empty list (since it's the first time) and it will keep being an empty list even after selecting some files (let's say "A" and "B") in the dialog, since it's associated to the current request the user is dealing with.

Then, if you open the dialog for the second time, selectedFiles will be filled now with the list of files previously selected ("A" and "B"), so the application will see them with the request in case it wants to do something with them (like pre-selecting them in the dialog). If now the user selects file "C" selectedFiles will internally keep containing "A" and "B", as won't be different until the next request, if that ever happens.

So this is precisely one of the reasons why we added the caching of selectedFiles at the level of the API (I mean, inside of WebKitFileChooserRequest, not in WebCore::FileChooser), so we could update the list of selected files at that level for the hypothetical case of the application calling to _get_selected_files() right after _select_files(), so we would return the list of files _currently_ selected even before disposing the current request, which would still be the old list in WebCore at that point.

Not sure if I managed to explain it properly. It's messy, I know... so feel free to ask again if you want some further clarification.

> So I moved to caching the selected files in 
> webkit_file_chooser_request_select_files() in the attached incremental
> patch. This fixes the test failure but then fails on a later test,
> where we select some files then click the button again and check that
> the selection was retained. This creates a new ChooserRequest object
> which has forgotten the selection of the first one.

It's ok to cache in the _select_files() method (so we can return the new list if the app calls _get_selected() right after making a new selection _and_ before disposing the current WebKitFileChooserRequest  object), but you should keep the cache in _get_selected_files() too, since you typically call it before opening the file chooser, so you know if you need to preselect some files in case there was a previous selection made.

> I then tried caching the initially selected files by reading the settings from
> webkit_file_chooser_request_create() but this segfaults upon access to
> chooser->settings(), not sure why.

I'm not sure either why this fails, nor why the previous tests you did fail too, but I'll take a look if possible today to it.

Btw, sorry for the late reply, but yesterday and today I'm attending to long meetings in the afternoon, so that's why I can't check this as promptly as I would like to.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.