Bug 83091

Created attachment 135448 [details] QML loader QML file showing the problem

Created attachment 135449 [details] HTML file with some mouse/links.

https://bugs.webkit.org/show_bug.cgi?id=83033 Seems to fix this.

(In reply to comment #0) > When loading an HTML file from QML without MiniBrowser, in "mobile" mode, links and mouse-events don't work. > Those start to work again when using touch-mocking, or when disabling Flickable. Subclassing from flickable should not change the behaviour for links so I wonder why it fixed your issue. In "mobile" mode on a device which has only touch events clicking on links happens with tap gestures. If this does not work with touch events then there is something wrong with the tap gesture recognizer. The mouse events are disabled for the flickable webview for now, since it would select text when panning, but after http://codereview.qt-project.org/21896 we can enable it again.

> Subclassing from flickable should not change the behaviour for links so I wonder why it fixed your issue. It didn't, I tested again. Might have been a testing error. > In "mobile" mode on a device which has only touch events clicking on links happens with tap gestures. There's no way to disable mobile mode > If this does not work with touch events then there is something wrong with the tap gesture recognizer. > > The mouse events are disabled for the flickable webview for now, since it would select text when panning, but after http://codereview.qt-project.org/21896 we can enable it again. So, right now, there's still no way to get pages with links running with pure QML... Let's see if that gets fixed after that Qt bug is in.

(In reply to comment #4) > The mouse events are disabled for the flickable webview for now, since it would select text when panning, but after http://codereview.qt-project.org/21896 we can enable it again. For the record: the mentioned change was abandoned in favor of another one https://codereview.qt-project.org/#change,24189

Created attachment 155290 [details] proposed patch

(In reply to comment #7) > Created an attachment (id=155290) [details] > proposed patch Simon, is this similar to what you had in mind when we were discussing this issue?

Comment on attachment 155290 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=155290&action=review Looks good in general, a couple of small issues and questions :) > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:460 > + isMouseEvent = true; > case QEvent::TouchBegin: I think between those two lines we usually add a "// Fall through" comment, to indicate that the "fall through" is deliberate. > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:463 > + m_isMouseButtonPressed = true; Shouldn't this be set in QEvent::MouseButtonPress? > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:476 > + isMouseEvent = true; > case QEvent::TouchUpdate: Fall through comment. > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:483 > + isMouseEvent = true; > case QEvent::TouchEnd: And here :) > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:484 > + m_isMouseButtonPressed = false; Same question as above: Should this be in case QEvent::MouseButtonRelease? > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:515 > + currentTouchPoint.setId(mouseEvent->buttons()); Maybe worth a comment, it's easy to overlook this trick when glancing through the code :) > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:517 > + currentTouchPoint.setRect(QRectF(mouseEvent->localPos(), QSizeF(40, 40))); Hm, why 40, 40 instead of 1, 1?

Created attachment 156077 [details] proposed patch

Comment on attachment 156077 [details] proposed patch r=me nice :)

Comment on attachment 156077 [details] proposed patch Clearing flags on attachment: 156077 Committed r124455: <http://trac.webkit.org/changeset/124455>

All reviewed patches have been landed. Closing bug.