[vlc-devel] [vlc-commits] Use QT file browser functions which return URLs when possible.

Hugo Beauzée-Luyssen hugo at beauzee.fr
Wed Dec 28 11:53:57 CET 2016


On 12/26/2016 04:21 PM, Frank Praznik wrote:
> vlc | branch: master | Frank Praznik <frank.praznik at gmail.com> | Wed Dec 21 14:04:02 2016 -0500| [4c85645838a12653f270667722f05ff714e648f5] | committer: Jean-Baptiste Kempf
>
> Use QT file browser functions which return URLs when possible.
>
> The getOpenFileNames() and getSaveFileName() methods of QFileDialog only work
> on local paths and return a blank string if the dialog is used to select a
> remote file (e.g., on a Samba share).  As of Qt 5.2 the QFileDialog class
> provides the methods getOpenFileUrls() and getSaveFileUrl() that return QUrl
> objects which can contain URLs to remote paths.  Use these methods when an
> appropriate version of the Qt libraries are available so that the paths to
> remote files are returned correctly when selected via the various file
> selection dialogs.
>
> Signed-off-by: Frank Praznik <frank.praznik at gmail.com>
> Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=4c85645838a12653f270667722f05ff714e648f5
> ---
>
>  modules/gui/qt/components/open_panels.cpp |  8 +++++-
>  modules/gui/qt/dialogs_provider.cpp       | 48 ++++++++++++++++++++++++++-----
>  modules/gui/qt/dialogs_provider.hpp       | 10 +++++++
>  modules/gui/qt/qt.hpp                     |  1 +
>  4 files changed, 59 insertions(+), 8 deletions(-)
>
> diff --git a/modules/gui/qt/components/open_panels.cpp b/modules/gui/qt/components/open_panels.cpp
> index 9f05548..08d4df0 100644
> --- a/modules/gui/qt/components/open_panels.cpp
> +++ b/modules/gui/qt/components/open_panels.cpp
> @@ -220,7 +220,7 @@ void FileOpenPanel::dropEvent( QDropEvent *event )
>
>  void FileOpenPanel::browseFile()
>  {
> -    QStringList files = QFileDialog::getOpenFileNames( this, qtr( "Select one or multiple files" ), p_intf->p_sys->filepath) ;
> +    QStringList files = DialogsProvider::getOpenFileNames( this, qtr( "Select one or multiple files" ), p_intf->p_sys->filepath );
>      foreach( const QString &file, files )
>      {
>          QListWidgetItem *item =
> @@ -273,9 +273,15 @@ void FileOpenPanel::updateMRL()
>          }
>      else
>      {
> +#if HAS_QT52
> +        QList<QUrl> urls = dialogBox->selectedUrls();
> +        foreach( const QUrl &url, urls )
> +            fileList.append( url.toEncoded() );
> +#else
>          fileList = dialogBox->selectedFiles();
>          for( int i = 0; i < fileList.count(); i++ )
>              fileList[i] = toURI( fileList[i] );
> +#endif
>      }
>
>      /* Options */
> diff --git a/modules/gui/qt/dialogs_provider.cpp b/modules/gui/qt/dialogs_provider.cpp
> index 0d37cdb..91852f2 100644
> --- a/modules/gui/qt/dialogs_provider.cpp
> +++ b/modules/gui/qt/dialogs_provider.cpp
> @@ -110,6 +110,40 @@ DialogsProvider::~DialogsProvider()
>      delete miscPopupMenu;
>  }
>
> +QStringList DialogsProvider::getOpenFileNames( QWidget *parent,
> +                                               const QString &caption,
> +                                               const QString &dir,
> +                                               const QString &filter,
> +                                               QString *selectedFilter )
> +{
> +    QStringList files;
> +
> +#if HAS_QT52
> +    QList<QUrl> urls = QFileDialog::getOpenFileUrls( parent, caption, QUrl::fromUserInput( dir ), filter, selectedFilter );
> +
> +    foreach( const QUrl url, urls )
> +        files.append( url.toEncoded() );
> +#else
> +    files = QFileDialog::getOpenFileNames( parent, caption, dir, filter, selectedFilter );
> +#endif
> +
> +    return files;
> +}
> +
> +QString DialogsProvider::getSaveFileName( QWidget *parent,
> +                                          const QString &caption,
> +                                          const QString &dir,
> +                                          const QString &filter,
> +                                          QString *selectedFilter )
> +{
> +#if HAS_QT52
> +    QUrl url = QFileDialog::getSaveFileUrl( parent, caption, QUrl::fromUserInput( dir ), filter, selectedFilter );
> +    return QString( url.toEncoded() );
> +#else
> +    return QFileDialog::getSaveFileName( parent, caption, dir, filter, selectedFilter );
> +#endif
> +}
> +
>  void DialogsProvider::quit()
>  {
>      b_isDying = true;
> @@ -349,7 +383,7 @@ void DialogsProvider::openFileGenericDialog( intf_dialog_args_t *p_arg )
>      /* Save */
>      if( p_arg->b_save )
>      {
> -        QString file = QFileDialog::getSaveFileName( NULL,
> +        QString file = getSaveFileName( NULL,
>                                          qfu( p_arg->psz_title ),
>                                          p_intf->p_sys->filepath, extensions );
>          if( !file.isEmpty() )
> @@ -363,7 +397,7 @@ void DialogsProvider::openFileGenericDialog( intf_dialog_args_t *p_arg )
>      }
>      else /* non-save mode */
>      {
> -        QStringList files = QFileDialog::getOpenFileNames( NULL,
> +        QStringList files = getOpenFileNames( NULL,
>                  qfu( p_arg->psz_title ), p_intf->p_sys->filepath,
>                  extensions );
>          p_arg->i_results = files.count();
> @@ -462,7 +496,7 @@ QStringList DialogsProvider::showSimpleOpen( const QString& help,
>      ADD_EXT_FILTER( fileTypes, EXTENSIONS_ALL );
>      fileTypes.replace( ";*", " *");
>
> -    QStringList files = QFileDialog::getOpenFileNames( NULL,
> +    QStringList files = getOpenFileNames( NULL,
>          help.isEmpty() ? qtr(I_OP_SEL_FILES ) : help,
>          path.isEmpty() ? p_intf->p_sys->filepath : path,
>          fileTypes );
> @@ -616,10 +650,10 @@ void DialogsProvider::saveAPlaylist(playlist_t *p_playlist, playlist_item_t *p_n
>      }
>
>      QString selected;
> -    QString file = QFileDialog::getSaveFileName( NULL,
> -                                                 qtr( "Save playlist as..." ),
> -                                                 p_intf->p_sys->filepath, filters.join( ";;" ),
> -                                                 &selected );
> +    QString file = getSaveFileName( NULL,
> +                                    qtr( "Save playlist as..." ),
> +                                    p_intf->p_sys->filepath, filters.join( ";;" ),
> +                                    &selected );
>      const char *psz_selected_module = NULL;
>      const char *psz_last_playlist_ext = NULL;
>
> diff --git a/modules/gui/qt/dialogs_provider.hpp b/modules/gui/qt/dialogs_provider.hpp
> index 36a41ea..b72f8ec 100644
> --- a/modules/gui/qt/dialogs_provider.hpp
> +++ b/modules/gui/qt/dialogs_provider.hpp
> @@ -91,6 +91,16 @@ public:
>                                  const QString& path = QString() );
>      bool isDying() { return b_isDying; }
>      static QString getDirectoryDialog( intf_thread_t *p_intf);
> +    static QStringList getOpenFileNames( QWidget *parent = Q_NULLPTR,
> +                                         const QString &caption = QString(),
> +                                         const QString &dir = QString(),
> +                                         const QString &filter = QString(),
> +                                         QString *selectedFilter = Q_NULLPTR );
> +    static QString getSaveFileName( QWidget *parent = Q_NULLPTR,
> +                                    const QString &caption = QString(),
> +                                    const QString &dir = QString(),
> +                                    const QString &filter = QString(),
> +                                    QString *selectedFilter = Q_NULLPTR );
>
>  protected:
>      QSignalMapper *menusMapper;
> diff --git a/modules/gui/qt/qt.hpp b/modules/gui/qt/qt.hpp
> index 99ce132..b666cbb 100644
> --- a/modules/gui/qt/qt.hpp
> +++ b/modules/gui/qt/qt.hpp
> @@ -47,6 +47,7 @@
>  #endif
>
>  #define HAS_QT5  ( QT_VERSION >= 0x050000 )
> +#define HAS_QT52 ( QT_VERSION >= 0x050200 )
>  #define HAS_QT56 ( QT_VERSION >= 0x050600 )
>
>  /* Q_DECL_OVERRIDE is a Qt5 feature, add empty define to not break with Qt4 */
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
>

This introduces a regression that prevents files to be opened on 
Windows. Basically now that the "file" is actually a URL, this code 
doesn't work anymore:
toURI( toNativeSeparators( file ) )

toNativeSeparator will convert scheme:// to scheme:\\, leading toURI to 
convert a formally valid URL into an invalid one.

Fixing the few places where toURI( toNativeSeparators( ... ) ) are being 
used together is easy, but it doesn't feel like a clean fix.

Any opinion on what the correct solution would be? I'd be in favor of 
making getOpenFileNames always return a URL, and fixing the callsites 
accordingly.

Regards,



More information about the vlc-devel mailing list