Bug 29389 – [Qt] isAccepted() from mouse event handling has a strange behavior on buttons of google.com

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[Tor Arne Vestbø]

This bug report originated from issue QTBUG-2959
<http://bugreports.qt.nokia.com/browse/QTBUG-2959>

--- Description ---

For webkit page showing www.google.com, clicking in the empty white areas produces mouse events that QWebPage marks as isAccepted(), whereas clicking on links and buttons produces mouse events that QWebPage marks as !isAccepted().

This seems barkwards.

[Joseph Ligman]

I can look at this bug. Not sure what the procedure is for unassigned bugs.

[Ariya Hidayat]

Joseph, any progress on this?

[Joseph Ligman]

(In reply to comment #2)
> Joseph, any progress on this?

No, sorry.

[Robert Hogan]

(In reply to comment #0)
> This bug report originated from issue QTBUG-2959
> <http://bugreports.qt.nokia.com/browse/QTBUG-2959>
> 
> --- Description ---
> 
> For webkit page showing www.google.com, clicking in the empty white areas
> produces mouse events that QWebPage marks as isAccepted(), whereas clicking on
> links and buttons produces mouse events that QWebPage marks as !isAccepted().
> 
> This seems barkwards.

I think it's correct - insofar as WebCore seems to be only interested in accepting mousePressEvents that initiate or manage a selection. Clicking links and buttons can't be the start of a selection in practice, no matter how hard you try. ;-)

bool EventHandler::handleMousePressEvent(const MouseEventWithHitTestResults& event)
{
<..>
    // If we got the event back, that must mean it wasn't prevented,
    // so it's allowed to start a drag or selection.
    m_mouseDownMayStartSelect = canMouseDownStartSelect(event.targetNode());
<..>
    bool swallowEvent = false;
    m_frame->selection()->setCaretBlinkingSuspended(true);
    m_mousePressed = true;
    m_beganSelectingText = false;

    if (event.event().clickCount() == 2)
        swallowEvent = handleMousePressEventDoubleClick(event);
    else if (event.event().clickCount() >= 3)
        swallowEvent = handleMousePressEventTripleClick(event);
    else
        swallowEvent = handleMousePressEventSingleClick(event);

    return swallowEvent;

}

// Whether or not a mouse down can begin the creation of a selection.  Fires the selectStart event.
bool EventHandler::canMouseDownStartSelect(Node* node)
{
    if (!node || !node->renderer())
        return true;
    
    // Some controls and images can't start a select on a mouse down.
    if (!node->canStartSelection())
        return false;
            
    for (RenderObject* curr = node->renderer(); curr; curr = curr->parent()) {
        if (Node* node = curr->node())
            return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
    }

    return true;
}

bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestResults& event)
{
    Node* innerNode = event.targetNode();
    if (!(innerNode && innerNode->renderer() && m_mouseDownMayStartSelect))
        return false;
<..>

}

[Yael]

This is going to be a problem for Orbit.
In Orbit, if you don't accept the MousePress event, you will not receive MouseMove or MouseRelease events.

[Benjamin Poulain]

Input element of button types behave as you would expect. The page google.com is not using the default behavior.
I have no idea what is going there.

[Rafael Brandao]

Could anyone provide an example that does not behave like that? I could reproduce it everywhere I tried. It's just like Robert said, it's only interested on such events that could start selection, so a button may not behave like that.

[Rafael Brandao]

QWebPage::event returns true if it has any function that handles that kind of event, no matter if the event has been accepted or not by the event handler:

bool QWebPage::event(QEvent *ev)
{
    switch (ev->type()) {
    case QEvent::Timer:
        d->timerEvent(static_cast<QTimerEvent*>(ev));
        break;
<..>
    default:
        return QObject::event(ev);
    }

    return true;
}

So I suppose it won't propagate this event to the parents. How could this flag isAccepted() make any difference in this case? Another thing I would like to ask, if QWebPage handle the event and then mark it as ignored to allow any parents to keep track of that event as well, how bad this could be?

[Rafael Brandao]

QGraphicsWebView and QWebView ignores the flag set by QWebPage as you can see above:

void QGraphicsWebView::mousePressEvent(QGraphicsSceneMouseEvent* ev)
{
    if (d->page) {
        const bool accepted = ev->isAccepted(); // Stores the original value.
        d->page->event(ev); // Handle the event with QWebPage.
        ev->setAccepted(accepted); // Restores the original accepted value.
    }

    if (!ev->isAccepted())
        QGraphicsItem::mousePressEvent(ev);
}


void QWebView::mousePressEvent(QMouseEvent* ev)
{
    if (d->page) {
        const bool accepted = ev->isAccepted();
        d->page->event(ev);
        ev->setAccepted(accepted);
    }
}

QWebView in particular never propagates the event, so I can't see how this bug could affect anything at all.

[Rafael Brandao]

In order to use QWebPage in a context that propagating mouse events could be relevant would be if you use it as a QWidget or a QGraphicsItem (correct me if I am wrong). Then you get QWebView and QGraphicsWebView.

Both of them ignore what QWebPage does with the event's accept flag as I've shown before, and they always accept the event, no matter where or what you click, as long as you click inside its area.

So what's the real point of changing the QWebPage's mousePressEvent accept flag in particular without changing how QWebView or QGraphicsWebView manages that change? Then I've checked the changelog looking for anything that would explain why QWebView always accept the event, and I've got this one:

2008-10-23  Simon Hausmann  <[e-mail]>

        Reviewed by Tor Arne.

        Fix handling of mouse events when embedding QWebView into the
        QGraphicsView.

        QWebPage accepts or ignores events to indicate whether the web page
        handled the event. However for QWebView to behave like a good widget
        it should always accept the mouse events to indicate that it handled
        them and that they should not be subject to event propagation.

        The graphics view relies on acceptance of the initial mouse click to
        make the embedded widget the focus item.

        * Api/qwebview.cpp:
        (QWebView::mouseMoveEvent):
        (QWebView::mousePressEvent):
        (QWebView::mouseDoubleClickEvent):
        (QWebView::mouseReleaseEvent):
        (QWebView::contextMenuEvent):
        (QWebView::wheelEvent):

If I got this right, it is expected that both QWebView and QGraphicsWebView accept that click event when it gets there.

Maybe the most appropriated bug fix for this one is to always accept events like mousePressEvent with QWebPage, as its widget (QWebView) or graphics item (QGraphicsWebView) does, so they would behave the same way and this would be more consistent.

Any opinions or observations are most certainly welcome.

[Rafael Brandao]

After a discussion on #qtwebkit, it was raised that this change should not be done as Apple bubbling events interest may be different and it could fail into some pretty uncommon test cases.