RESOLVED FIXED 50419
[Qt] QWebPage sends out a RequestSoftwareInputPanel event incorrectly in particular case
https://bugs.webkit.org/show_bug.cgi?id=50419
Summary [Qt] QWebPage sends out a RequestSoftwareInputPanel event incorrectly in part...
Yi Shen
Reported 2010-12-02 16:31:23 PST
For a page like, <input type='text' id='input'/> <div onclick="i=document.getElementById('input'); i.focus();">abc</div> If user click on the div, the javascript sets the focus on the text input field. Then QWebPage sends out a RequestSoftwareInputPanel event, which invokes a VKB on symbian device. void QWebPagePrivate::mouseReleaseEvent(T *ev) { ... ... if (mev.button() != NoButton) accepted = frame->eventHandler()->handleMouseReleaseEvent(mev); // set focus on the text input field which enable the inputmethod ... ... handleSoftwareInputPanel(ev->button()); // emit RequestSoftwareInputPanel event since the inputmethod has been enabled }
Attachments
Add HitTestResult check before firing the RequestSoftwareInputPanel event (4.18 KB, patch)
2010-12-02 16:40 PST, Yi Shen
eric: review-
add help function for testing (7.74 KB, patch)
2010-12-10 19:30 PST, Yi Shen
no flags
Yi Shen
Comment 1 2010-12-02 16:40:35 PST
Created attachment 75429 [details] Add HitTestResult check before firing the RequestSoftwareInputPanel event
Eric Seidel (no email)
Comment 2 2010-12-02 17:29:51 PST
Comment on attachment 75429 [details] Add HitTestResult check before firing the RequestSoftwareInputPanel event View in context: https://bugs.webkit.org/attachment.cgi?id=75429&action=review > WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:2022 > + QMouseEvent evpres(QEvent::MouseButtonPress, inputElement.geometry().center(), Qt::LeftButton, Qt::NoButton, Qt::NoModifier); > + page->event(&evpres); > + QMouseEvent evrel(QEvent::MouseButtonRelease, inputElement.geometry().center(), Qt::LeftButton, Qt::NoButton, Qt::NoModifier); > + page->event(&evrel); Seems the testing infrastructure should have some helper methods for doing clicks like this. This is insanely verbose. :)
Eric Seidel (no email)
Comment 3 2010-12-09 23:39:21 PST
Comment on attachment 75429 [details] Add HitTestResult check before firing the RequestSoftwareInputPanel event r- for insane verbosity. Please add a helper method.
Yi Shen
Comment 4 2010-12-10 06:29:50 PST
(In reply to comment #3) > (From update of attachment 75429 [details]) > r- for insane verbosity. Please add a helper method. Thanks for reviewing, eric :) I was wondering whether I should add the helper method in the patch. Do I need to make a new bug for adding the helper method, which can service all the qt api tests under the webkit/qt/tests folder?
Yi Shen
Comment 5 2010-12-10 19:30:38 PST
Created attachment 76289 [details] add help function for testing
Andreas Kling
Comment 6 2010-12-16 08:27:40 PST
Kenneth is our new expert on Input Method stuff, CC'ing him. :)
Kenneth Rohde Christiansen
Comment 7 2010-12-16 08:36:20 PST
Comment on attachment 76289 [details] add help function for testing View in context: https://bugs.webkit.org/attachment.cgi?id=76289&action=review > WebKit/qt/Api/qwebpage.cpp:762 > + handleSoftwareInputPanel(ev->button(), QPointF(ev->pos()).toPoint()); I think this should be the point where the mouse was pressed, not released. You can click, hold and select text and then release and it should popup with a selection. Probably needs a comment for that. Anyway, I guess instead of doing a hit test what you want to do is to check if the selection (caret or range) is editable. You should look at the inputMethodQuery methods. Something like frame->selection()->isContentEditable().
Yi Shen
Comment 8 2010-12-16 10:56:28 PST
(In reply to comment #7) > (From update of attachment 76289 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=76289&action=review > > > WebKit/qt/Api/qwebpage.cpp:762 > > + handleSoftwareInputPanel(ev->button(), QPointF(ev->pos()).toPoint()); > > I think this should be the point where the mouse was pressed, not released. You can click, hold and select text and then release and it should popup with a selection. > > Probably needs a comment for that. Anyway, I guess instead of doing a hit test what you want to do is to check if the selection (caret or range) is editable. You should look at the inputMethodQuery methods. Something like frame->selection()->isContentEditable(). Thanks Kenneth. For following page, <input type='text' id='input'/> <div onclick="i=document.getElementById('input');i.focus(); i.select();">abc</div> The VKB still pops up when user click on the div, if we only check the frame->selection()->isContentEditable(), right? Correct me if I am wrong.
Yi Shen
Comment 9 2010-12-16 13:34:41 PST
(In reply to comment #7) > (From update of attachment 76289 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=76289&action=review > > > WebKit/qt/Api/qwebpage.cpp:762 > > + handleSoftwareInputPanel(ev->button(), QPointF(ev->pos()).toPoint()); > > I think this should be the point where the mouse was pressed, not released. You can click, hold and select text and then release and it should popup with a selection. > > Probably needs a comment for that. Anyway, I guess instead of doing a hit test what you want to do is to check if the selection (caret or range) is editable. You should look at the inputMethodQuery methods. Something like frame->selection()->isContentEditable(). Kenneth, I just tried to use frame->selection()->isContentEditable() to determine whether or not send requestsoftwareinputpanel event, but it failed my test case. See my changed code below, void QWebPagePrivate::handleSoftwareInputPanel(...) { ... if (frame->selection()->isContentEditable()) { QEvent event(QEvent::RequestSoftwareInputPanel); QApplication::sendEvent(client->ownerWidget(), &event); } } But I am agree with you about using the mouse press position, instead of release position (This is how iphone safari does). I think we should have a separated bug for that since it may involve more discussion, e.g. platform behavior. For this bug, let's just focus on fixing this particular issue :)
WebKit Commit Bot
Comment 10 2010-12-16 13:42:32 PST
Comment on attachment 76289 [details] add help function for testing Rejecting attachment 76289 [details] from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76289]" exit_code: 2 Last 500 characters of output: ailed to merge in the changes. Patch failed at 0001 2010-12-16 Yury Semikhatsky <yurys@chromium.org> When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at WebKitTools/Scripts/update-webkit line 132. Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2 Full output: http://queues.webkit.org/results/7115064
Yi Shen
Comment 11 2010-12-16 14:18:29 PST
(In reply to comment #10) > (From update of attachment 76289 [details]) > Rejecting attachment 76289 [details] from commit-queue. > > Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-attachment', '--non-interactive', 76289]" exit_code: 2 > Last 500 characters of output: > ailed to merge in the changes. > Patch failed at 0001 2010-12-16 Yury Semikhatsky <yurys@chromium.org> > > When you have resolved this problem run "git rebase --continue". > If you would prefer to skip this patch, instead run "git rebase --skip". > To restore the original branch and stop rebasing run "git rebase --abort". > > rebase refs/remotes/origin/master: command returned error: 1 > > Died at WebKitTools/Scripts/update-webkit line 132. > > Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2 > > Full output: http://queues.webkit.org/results/7115064 Kenneth, is this a buildbot issue? I have another patch got the same error output...
Eric Seidel (no email)
Comment 12 2010-12-16 14:34:36 PST
Comment on attachment 76289 [details] add help function for testing The git repo on one of the commit-queues got corrupted. I'mf ixing it now. Sorry for the noise!
WebKit Commit Bot
Comment 13 2010-12-16 19:21:25 PST
The commit-queue encountered the following flaky tests while processing attachment 76289 [details]: fast/loader/recursive-before-unload-crash.html bug 50880 (authors: beidson@apple.com and eric@webkit.org) fast/preloader/script.html bug 50879 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 14 2010-12-16 22:56:34 PST
The commit-queue encountered the following flaky tests while processing attachment 76289 [details]: editing/selection/caret-mode-paragraph-keys-navigation.html bug 51234 (author: tonikitoo@webkit.org) compositing/reflections/animation-inside-reflection.html bug 50875 (author: simon.fraser@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 15 2010-12-17 01:26:29 PST
The commit-queue encountered the following flaky tests while processing attachment 76289 [details]: fast/images/load-img-with-empty-src.html bug 50855 (author: mitz@webkit.org) http/tests/misc/redirect.php bug 51238 (author: mjs@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 16 2010-12-17 02:51:48 PST
Comment on attachment 76289 [details] add help function for testing Clearing flags on attachment: 76289 Committed r74243: <http://trac.webkit.org/changeset/74243>
WebKit Commit Bot
Comment 17 2010-12-17 02:51:56 PST
All reviewed patches have been landed. Closing bug.
Ademar Reis
Comment 18 2011-01-24 13:13:38 PST
Revision r74243 cherry-picked into qtwebkit-2.2 with commit 01fd783 <http://gitorious.org/webkit/qtwebkit/commit/01fd783>
Note You need to log in before you can comment on or make changes to this bug.