[vlc-devel] [PATCH 16/18] gui/qt: fix #17184 (missing entries in context-menu)

[Filip Roséen]

On 16/07/20 11:25, Francois Cartegnie wrote:
>      Actions that are not bound to a single entity are now displayed
>      correctly (vlc 2.2.4 has been used as reference), with two minor
>      differences:
>          * ACTION_PAUSE will now be shown if the current item is being
>            played, and;
>          * ACTION_PLAY will not show if the item is already playing.
>      -    if( !index.isValid() )
>      -        return false;
>      -
>      -    const PLItem *item = getItem( index );
>      +    vlc_playlist_locker pl_lock ( THEPL );
>      +        
>      +    bool const b_readonly = THEPL->p_root->i_flags & PLAYLIST_RO_FLAG;
>      +    AbstractPLItem * item = VLCModel::getItem( index );
>      And then you introduce NULL deref in most cases.

Could you please point to a case where "NULL deref" happens might/will
happen? I might be missing something very trivial, but I honestly do not
see it.
>      +        case ACTION_EXPLORE:
>      +        {
>      +            return getURI( index ).startsWith( "file:///" );
>      +        }
>      You changed scheme without reason.

What scheme has been changed? the logic for ACTION_EXPLORE is the same as
before. If you are talking about the introduction of "{ }", I did such
because of habits when writing switch-cases (to have symmetry between the
different cases)

I was not aware of a policy saying that such should not be used for
single-line handlers; and if you feel strongly that it should not be I have
no problem with removing such.
>      +        case ACTION_RENAMENODE:
>      +        {
>      +            if( index == rootIndex() || item == NULL )
>      +                return false;
>      +
>      +            input_item_t* p_iitem = item->inputItem();
>      +
>      +            if( p_iitem == NULL )
>      +                return false;
>      +
>      +            return p_iitem->i_type == ITEM_TYPE_NODE
>      +                || p_iitem->i_type == ITEM_TYPE_DIRECTORY;
>      +        }
>      You removed readonly cases.
>      You're not allowed to rename readonly nodes.

Oh, you are right; I will fix this as soon as possible!

Though I do not see any other case where the usage of "b_readonly" vs
"item->readOnly()" has been left out.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160720/01f3e284/attachment.html>