[vlc-devel] [PATCH 1/3] gui/qt: refactor handling of drag-drop-play

[Filip Roséen]

Hi Hugo,

On 2017-01-16 12:54, Hugo Beauzée-Luyssen wrote:

> On 01/12/2017 07:34 PM, Filip Roséen wrote:
> > The previous implementation contained duplicate functionality in
> > different code-paths, as well as suffering from a bug where a
> > potential subtitle-drop would not be probed for such.
> > 
> > These changes simplifies the implementation, as well as making sure
> > that we treat a drop of one (1) URL the same, no matter if the
> > mime-type is stating that it is a URI, or text/* (such as when dragged
> > from a web browser).
> > 
> > One should also be able to drag-and-drop entities that are referring
> > to non-local entities, as such that check has been removed.
> > 
> > refs #17657
> > ---
> >  modules/gui/qt/main_interface.cpp | 64 ++++++++++++---------------------------
> >  1 file changed, 19 insertions(+), 45 deletions(-)
> > 
> > diff --git a/modules/gui/qt/main_interface.cpp b/modules/gui/qt/main_interface.cpp
> > index 894da7afd6..810fdbdf8d 100644
> > --- a/modules/gui/qt/main_interface.cpp
> > +++ b/modules/gui/qt/main_interface.cpp
> > @@ -1420,58 +1420,32 @@ void MainInterface::dropEventPlay( QDropEvent *event, bool b_play, bool b_playli
> >      else
> >          return;
> > 
> > -    const QMimeData *mimeData = event->mimeData();
> > +    QMimeData const* md = event->mimeData();
> > +    QList<QUrl> urls = md->urls();
> > 
> > -    /* D&D of a subtitles file, add it on the fly */
> > -    if( mimeData->urls().count() == 1 && THEMIM->getIM()->hasInput() )
> > -    {
> > -        if( !input_AddSubtitleOSD( THEMIM->getInput(),
> > -                 qtu( toNativeSeparators( mimeData->urls()[0].toLocalFile() ) ),
> > -                 true, true ) )
> > -        {
> > -            event->accept();
> > -            return;
> > -        }
> > -    }
> > +    if( urls.count() == 0 && md->hasText() )
> > +        urls << QUrl( md->text() );
> 
> Shouldn't this be QUrl::fromLocalFile?

The dragged entity is not guaranteed to refer to a local file, which
is why `QUrl::fromLocalFile` is not used.

> > 
> > -    bool first = b_play;
> > -    foreach( const QUrl &url, mimeData->urls() )
> > +    if( urls.count() == 1 && urls.back().isValid() &&
> > +        input_AddSubtitleOSD( THEMIM->getInput(),
> > +                              qtu( urls.back().toString( QUrl::FullyEncoded ) ),
> > +                              true, true ) == VLC_SUCCESS )
> >      {
> > -        if( url.isValid() )
> > -        {
> > -            QString mrl = toURI( url.toEncoded().constData() );
> > -            QFileInfo info( url.toLocalFile() );
> > -            if( info.exists() && info.isSymLink() )
> > -            {
> > -                QString target = info.symLinkTarget();
> > -                QUrl url;
> > -                if( QFile::exists( target ) )
> > -                {
> > -                    url = QUrl::fromLocalFile( target );
> > -                }
> > -                else
> > -                {
> > -                    url.setUrl( target );
> > -                }
> > -                mrl = toURI( url.toEncoded().constData() );
> > -            }
> > -            if( mrl.length() > 0 )
> > -            {
> > -                Open::openMRL( p_intf, mrl, first, b_playlist );
> > -                first = false;
> > -            }
> > -        }
> > +        event->accept();
> > +        return;
> >      }
> > 
> > -    /* Browsers give content as text if you dnd the addressbar,
> > -       so check if mimedata has valid url in text and use it
> > -       if we didn't get any normal Urls()*/
> > -    if( !mimeData->hasUrls() && mimeData->hasText() &&
> > -        QUrl(mimeData->text()).isValid() )
> > +    foreach( const QUrl& url, urls )
> >      {
> > -        QString mrl = toURI( mimeData->text() );
> > -        Open::openMRL( p_intf, mrl, first, b_playlist );
> > +        if( url.isValid() == false )
> > +            continue;
> > +
> > +        Open::openMRL( p_intf, url.toString( QUrl::FullyEncoded ),
> > +                       b_play, b_playlist );
> > +
> > +        b_play = false; /* only immediately play the first entry */
> >      }
> > +
> >      event->accept();
> >  }
> >  void MainInterface::dragEnterEvent(QDragEnterEvent *event)
> > 
> 
> Unrelated to the commit itself, but why do we have a specific handling for a single
> file, and a different one for multiple files? Shouldn't it use the same code?

We do not want to try to add things dragged as a subtitle if more than
one entity is dragged, and that's why the code is written the way it
is.
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170123/780f9727/attachment.html>