With latest Qt and latest webkit, cannot type username/password in HTTP Authentication dialog. Try loading e.g. http://iop4.nokia-boston.com. The Authentication dialog shows, but users cannot type in it.
The mouse press event on the text input field is translated to touch event in MiniBrowser. This results in qquickwebview getting focus (it calls forceActiveFocus in touchEvent handler). Hence, the text input field loses focus and doesn't take any input.
I have a patch preventing event conversion in MiniBrowser if dialogRunner is active. Needed to expose an API from webView experimental to indicate this.
Created attachment 129734 [details] Prevent coverting mouse events to touch events in MiniBrowser if dialogRunner is active
Comment on attachment 129734 [details] Prevent coverting mouse events to touch events in MiniBrowser if dialogRunner is active I don't like that we have to work around our touch mocking. I'm not even sure this workaround is needed anymore now that we send the event synchronously, but ultimately the fix belongs into QtQuick to make MouseArea accept touch events (see discussion on qt development list).
(In reply to comment #4) > (From update of attachment 129734 [details]) > I don't like that we have to work around our touch mocking. I'm not even sure this workaround is needed anymore now that we send the event synchronously, but ultimately the fix belongs into QtQuick to make MouseArea accept touch events (see discussion on qt development list). I should have updated the bug. With the changes for sending the event synchronously, it is not an issue just with the input editor anymore. None of the dialogs can even be dismissed/accepted because the mouse events are not sent to the dialog item as QQuickWebView accepts the touch event. Agreed that QQuickItems should handle touch events (we have been waiting for such a fix for quite some time :) as there are other issues like QQuickWebView receiving touch events which can possibly cause a link to be selected etc. ). On hardware the input editor should still have an issue as QQuickWebView would grab the focus (I added the work-around in MiniBrowser thinking that this is limited to only touch mocking). The touch event handler in QQuickWebView should grab focus only if there is no active dialog (agreed this too is a work-around in the absence of a Qt fix).
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 129734 [details] [details]) > > I don't like that we have to work around our touch mocking. I'm not even sure this workaround is needed anymore now that we send the event synchronously, but ultimately the fix belongs into QtQuick to make MouseArea accept touch events (see discussion on qt development list). > > I should have updated the bug. With the changes for sending the event synchronously, it is not an issue just with the input editor anymore. None of the dialogs can even be dismissed/accepted because the mouse events are not sent to the dialog item as QQuickWebView accepts the touch event. Agreed that QQuickItems should handle touch events (we have been waiting for such a fix for quite some time :) as there are other issues like QQuickWebView receiving touch events which can possibly cause a link to be selected etc. ). > > On hardware the input editor should still have an issue as QQuickWebView would grab the focus (I added the work-around in MiniBrowser thinking that this is limited to only touch mocking). The touch event handler in QQuickWebView should grab focus only if there is no active dialog (agreed this too is a work-around in the absence of a Qt fix). Right. We are also in discussion with the QQuick folks about the event handling, and in the meantime it seems the best "workaround" is to not accept touch events while the dialogrunner is running.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 129734 [details] [details] [details]) > > > I don't like that we have to work around our touch mocking. I'm not even sure this workaround is needed anymore now that we send the event synchronously, but ultimately the fix belongs into QtQuick to make MouseArea accept touch events (see discussion on qt development list). > > > > I should have updated the bug. With the changes for sending the event synchronously, it is not an issue just with the input editor anymore. None of the dialogs can even be dismissed/accepted because the mouse events are not sent to the dialog item as QQuickWebView accepts the touch event. Agreed that QQuickItems should handle touch events (we have been waiting for such a fix for quite some time :) as there are other issues like QQuickWebView receiving touch events which can possibly cause a link to be selected etc. ). > > > > On hardware the input editor should still have an issue as QQuickWebView would grab the focus (I added the work-around in MiniBrowser thinking that this is limited to only touch mocking). The touch event handler in QQuickWebView should grab focus only if there is no active dialog (agreed this too is a work-around in the absence of a Qt fix). > > Right. We are also in discussion with the QQuick folks about the event handling, and in the meantime it seems the best "workaround" is to not accept touch events while the dialogrunner is running. I will modify the patch to add the check in QQuickWebView rather than in MiniBrowser.
Created attachment 130628 [details] Updated Patch
Comment on attachment 130628 [details] Updated Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130628&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:285 > + dialogRunner = new QtDialogRunner; This should use an OwnPtr and you should make a PassOwnPtr QtDialogRunner::create() > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:518 > +void QQuickWebViewPrivate::destroyDialogRunner() > +{ > + delete dialogRunner; > + dialogRunner = 0; > +} ieck! this calls for an OwnPtr
(In reply to comment #9) > (From update of attachment 130628 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130628&action=review > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:285 > > + dialogRunner = new QtDialogRunner; > > This should use an OwnPtr and you should make a PassOwnPtr QtDialogRunner::create() > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:518 > > +void QQuickWebViewPrivate::destroyDialogRunner() > > +{ > > + delete dialogRunner; > > + dialogRunner = 0; > > +} > > ieck! this calls for an OwnPtr Ah, didn't think about it :( Will change it.
Created attachment 130656 [details] Updated with Review Comments
Comment on attachment 130656 [details] Updated with Review Comments View in context: https://bugs.webkit.org/attachment.cgi?id=130656&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1385 > + if (d->dialogRunner.get()) > + event->ignore(); > + else { > + forceActiveFocus(); > + d->pageView->eventHandler()->handleTouchEvent(event); > + } I think webkit style would be an early return, i.e. if (showingDialog) return; forceActiveFocus(); d->pageView->eventHandler()->handleTouchEvent(event); > Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:157 > + OwnPtr<QtDialogRunner> dialogRunner; Do we really need all that ownptr, etc. and all the resulting cosmetic code changes if all we want to know is if we're showing the runner? Why not simply put a boolean in place?
(In reply to comment #12) > (From update of attachment 130656 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130656&action=review > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1385 > > + if (d->dialogRunner.get()) > > + event->ignore(); > > + else { > > + forceActiveFocus(); > > + d->pageView->eventHandler()->handleTouchEvent(event); > > + } > > I think webkit style would be an early return, i.e. > > if (showingDialog) > return; > > forceActiveFocus(); > d->pageView->eventHandler()->handleTouchEvent(event); > > > Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:157 > > + OwnPtr<QtDialogRunner> dialogRunner; > > Do we really need all that ownptr, etc. and all the resulting cosmetic code changes if all we want to know is if we're showing the runner? Why not simply put a boolean in place? I though of adding a bool but went the way of making dialogRunner a pointer so that we don't need to set the boolean every time we create a dialogRunner.
(In reply to comment #12) > (From update of attachment 130656 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=130656&action=review > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1385 > > + if (d->dialogRunner.get()) > > + event->ignore(); > > + else { > > + forceActiveFocus(); > > + d->pageView->eventHandler()->handleTouchEvent(event); > > + } > > I think webkit style would be an early return, i.e. > > if (showingDialog) > return; > > forceActiveFocus(); > d->pageView->eventHandler()->handleTouchEvent(event); Need to explicitly ignore the event as it is set to accepted (by Qt) when it is received here. > > Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:157 > > + OwnPtr<QtDialogRunner> dialogRunner; > > Do we really need all that ownptr, etc. and all the resulting cosmetic code changes if all we want to know is if we're showing the runner? Why not simply put a boolean in place?
Created attachment 130670 [details] Updated Patch
Comment on attachment 130670 [details] Updated Patch That looks indeed much simpler to me :-) All those chunks of duplicated code when executing the dialogs smell like they could be simplified, but that's completely unrelated to this bug.
(In reply to comment #16) > (From update of attachment 130670 [details]) > That looks indeed much simpler to me :-) > > All those chunks of duplicated code when executing the dialogs smell like they could be simplified, but that's completely unrelated to this bug. Yes. I agree. In fact for authentication and proxy authentication I was going to do a patch as that is almost entirely duplicated.
Comment on attachment 130670 [details] Updated Patch Clearing flags on attachment: 130670 Committed r110098: <http://trac.webkit.org/changeset/110098>
All reviewed patches have been landed. Closing bug.