D16643: Correct the accept flag of the event object on DragMove

[Stefan Brüns]

bruns added a comment.


  In D16643#405468 <https://phabricator.kde.org/D16643#405468>, @fvogt wrote:
  
  > > The second change fvogt mentioned is IMHO handled quite strangely here - !m_enabled || m_temporaryInhibition is the inverse of !m_enabled || m_temporaryInhibition, so doing a event->ignore() === event->setAccepted(false) prior to the first return statement would cover the ignored case, and allows the event to bubble up.
  >
  > Sure, but AFAICT that's just a matter of taste whether to `ignore(); if(!something) return; do stuff; accept();` or `accept(something); if(!something) return; do stuff;`.
  
  
  Its about the duplicate evaluation of ` !m_enabled || m_temporaryInhibition`. Expecially as it is not easy to spot both conditions are exactly the same.
  
  compare
  
    event->setAccepted(m_enabled && !m_temporaryInhibition);
    if (!m_enabled || m_temporaryInhibition) {
        return;
    }
  
  with
  
    if (!m_enabled || m_temporaryInhibition) {
        event->ignore();
        return;
    }
    event->accept();

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D16643

To: trmdi, mart, broulik, #plasma, hein, bruns
Cc: fvogt, aacid, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190205/99a4f8f6/attachment.html>