[Sugar-devel] [PATCH Browse] Restore journal file picker (#3411)

[Manuel Quiñones]

Thanks Daniel, this patch works for me in olpc build os13 installing
the webkitgtk test rpm [1]. Seems that you are close in landing the
patch upstream [2].

[1] http://dev.laptop.org/~dsd/20120604/
[2] https://bugs.webkit.org/show_bug.cgi?id=87283#c23

Patch looks good for me, just two comments below, no issues.  I'm
happy to apply it.

2012/6/4 Daniel Drake <dsd at laptop.org>:
> The run-file-chooser signal can be used to intercept a request
> for a file upload and display a custom file chooser.
>
> We want to do this to allow the user to upload files from the journal.

I think "from the journal" is a good simplification of "from the sugar
object chooser".  I'm fine with that.

> By default, the standard GTK+ file chooser was being shown.
>
> run-file-chooser is present in WebKit2 and has been submitted for
> final review for inclusion in WebKit1.
> ---
>  browser.py     |   19 ++++++++++
>  filepicker.py  |  107 ++++++--------------------------------------------------
>  webactivity.py |    1 -
>  webtoolbar.py  |    6 ++--
>  4 files changed, 32 insertions(+), 101 deletions(-)
>
> diff --git a/browser.py b/browser.py
> index 764f913..0e46585 100644
> --- a/browser.py
> +++ b/browser.py
> @@ -34,6 +34,7 @@ from sugar3.graphics import style
>  from sugar3.graphics.icon import Icon
>
>  from widgets import BrowserNotebook
> +from filepicker import FilePicker
>  import globalhistory
>  import downloadmanager
>  from pdfviewer import PDFTabPage
> @@ -456,6 +457,12 @@ class Browser(WebKit.WebView):
>         self.connect('new-window-policy-decision-requested',
>                      self.__new_window_policy_cb)
>
> +        try:
> +            self.connect('run-file-chooser', self.__run_file_chooser)
> +        except TypeError:
> +            # Only present in WebKit1 > 1.9.3 and WebKit2
> +            pass
> +
>     def get_history(self):
>         """Return the browsing history of this browser."""
>         back_forward_list = self.get_back_forward_list()
> @@ -529,6 +536,18 @@ class Browser(WebKit.WebView):
>     def open_new_tab(self, url):
>         self.emit('new-tab', url)
>
> +    def __run_file_chooser(self, browser, request):
> +        picker = FilePicker(self)
> +        chosen = picker.run()
> +        picker.destroy()
> +
> +        if chosen:
> +            request.select_files([chosen])
> +        elif hasattr(request, 'cancel'):
> +            # WebKit2 only
> +            request.cancel()
> +        return True
> +
>     def __load_status_changed_cb(self, widget, param):
>         """Add the url to the global history or update it."""
>         status = widget.get_load_status()
> diff --git a/filepicker.py b/filepicker.py
> index 489570a..65ac464 100644
> --- a/filepicker.py
> +++ b/filepicker.py
> @@ -20,12 +20,6 @@ import tempfile
>  import shutil
>
>  from gi.repository import Gtk
> -import hulahop
> -
> -import xpcom
> -from xpcom import components
> -from xpcom.components import interfaces
> -from xpcom.server.factory import Factory
>
>  from sugar3.graphics.objectchooser import ObjectChooser
>  from sugar3.activity.activity import get_activity_root
> @@ -44,114 +38,35 @@ def cleanup_temp_files():
>                           temp_dir)
>
>
> -class FilePicker:
> -    _com_interfaces_ = interfaces.nsIFilePicker
> -
> -    cid = '{57901c41-06cb-4b9e-8258-37323327b583}'
> -    description = 'Sugar File Picker'
> -
> -    def __init__(self):
> -        self._title = None
> -        self._parent = None
> -        self._file = None
> -
> -    def appendFilter(self, title, filter):
> -        logging.warning('FilePicker.appendFilter: UNIMPLEMENTED')
> -
> -    def appendFilters(self, filterMask):
> -        logging.warning('FilePicker.appendFilters: UNIMPLEMENTED')
> -
> -    def init(self, parent, title, mode):
> -        self._title = title
> -        self._file = None
> -        self._parent = hulahop.get_view_for_window(parent).get_toplevel()
> +class FilePicker(ObjectChooser):
> +    def __init__(self, parent):
> +        ObjectChooser.__init__(self, parent)
>
> -        if mode != interfaces.nsIFilePicker.modeOpen:
> -            raise xpcom.COMException(NS_ERROR_NOT_IMPLEMENTED)
> -
> -    def show(self):
> -        chooser = ObjectChooser(parent=self._parent)
> +    def run(self):
>         jobject = None
> +        _file = None
>         try:
> -            result = chooser.run()
> +            result = ObjectChooser.run(self)
>             if result == Gtk.ResponseType.ACCEPT:
> -                jobject = chooser.get_selected_object()
> +                jobject = self.get_selected_object()
>                 logging.debug('FilePicker.show: %r', jobject)
>
>                 if jobject and jobject.file_path:
>                     tmp_dir = tempfile.mkdtemp(prefix='', \
>                             dir=os.path.join(get_activity_root(), 'tmp'))
> -                    self._file = os.path.join(tmp_dir,
> -                            _basename_strip(jobject))
> +                    _file = os.path.join(tmp_dir, _basename_strip(jobject))
>
> -                    os.rename(jobject.file_path, self._file)
> +                    os.rename(jobject.file_path, _file)
>
>                     global _temp_dirs_to_clean
>                     _temp_dirs_to_clean.append(tmp_dir)
>
> -                    logging.debug('FilePicker.show: file=%r', self._file)
> +                    logging.debug('FilePicker.show: file=%r', _file)

I see you changed self._file for _file.  I guess you kept the
underscore so the variable name don't clash with the python special
name.

>         finally:
>             if jobject is not None:
>                 jobject.destroy()
> -            chooser.destroy()
> -            del chooser
> -
> -        if self._file:
> -            return interfaces.nsIFilePicker.returnOK
> -        else:
> -            return interfaces.nsIFilePicker.returnCancel
> -
> -    def set_defaultExtension(self, default_extension):
> -        logging.warning('FilePicker.set_defaultExtension: UNIMPLEMENTED')
> -
> -    def get_defaultExtension(self):
> -        logging.warning('FilePicker.get_defaultExtension: UNIMPLEMENTED')
> -        return None
> -
> -    def set_defaultString(self, default_string):
> -        logging.warning('FilePicker.set_defaultString: UNIMPLEMENTED')
> -
> -    def get_defaultString(self):
> -        logging.warning('FilePicker.get_defaultString: UNIMPLEMENTED')
> -        return None
> -
> -    def set_displayDirectory(self, display_directory):
> -        logging.warning('FilePicker.set_displayDirectory: UNIMPLEMENTED')
> -
> -    def get_displayDirectory(self):
> -        logging.warning('FilePicker.get_displayDirectory: UNIMPLEMENTED')
> -        return None
> -
> -    def set_filterIndex(self, filter_index):
> -        logging.warning('FilePicker.set_filterIndex: UNIMPLEMENTED')
> -
> -    def get_filterIndex(self):
> -        logging.warning('FilePicker.get_filterIndex: UNIMPLEMENTED')
> -        return None
> -
> -    def get_file(self):
> -        logging.debug('FilePicker.get_file: %r', self._file)
> -        if self._file:
> -            cls = components.classes["@mozilla.org/file/local;1"]
> -            local_file = cls.createInstance(interfaces.nsILocalFile)
> -            local_file.initWithPath(self._file)
> -            return local_file
> -        else:
> -            return None
> -
> -    def get_Files(self):
> -        logging.warning('FilePicker.get_Files: UNIMPLEMENTED')
> -        return None
> -
> -    def get_FileURL(self):
> -        logging.warning('FilePicker.get_FileURL: UNIMPLEMENTED')
> -        return None
> -
>
> -components.registrar.registerFactory(FilePicker.cid,
> -                                        FilePicker.description,
> -                                        '@mozilla.org/filepicker;1',
> -                                        Factory(FilePicker))
> +        return _file
>
>
>  def _basename_strip(jobject):
> diff --git a/webactivity.py b/webactivity.py
> index fe01754..2cf70a6 100644
> --- a/webactivity.py
> +++ b/webactivity.py
> @@ -134,7 +134,6 @@ from viewtoolbar import ViewToolbar
>  import downloadmanager
>
>  # TODO: make the registration clearer SL #3087
> -# import filepicker  # pylint: disable=W0611
>
>  from model import Model
>  from sugar3.presence.tubeconn import TubeConnection
> diff --git a/webtoolbar.py b/webtoolbar.py
> index e7c66c9..dcea998 100644
> --- a/webtoolbar.py
> +++ b/webtoolbar.py
> @@ -30,8 +30,7 @@ from sugar3.graphics.toolbarbox import ToolbarBox as ToolbarBase
>  from sugar3.activity.widgets import ActivityToolbarButton
>  from sugar3.activity.widgets import StopButton
>
> -# FIXME
> -# import filepicker
> +import filepicker
>  import places
>  from sugarmenuitem import SugarMenuItem
>  from browser import Browser
> @@ -424,8 +423,7 @@ class PrimaryToolbar(ToolbarBase):
>     def __uri_changed_cb(self, widget, param):
>         self._set_address(widget.get_uri())
>         self._update_navigation_buttons()
> -        # FIXME
> -        # filepicker.cleanup_temp_files()
> +        filepicker.cleanup_temp_files()
>
>     def _stop_and_reload_cb(self, entry, icon_pos, button):
>         if self._loading:
> --
> 1.7.10.2
>
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel



-- 
.. manuq ..