WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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-
Details
Formatted Diff
Diff
add help function for testing
(7.74 KB, patch)
2010-12-10 19:30 PST
,
Yi Shen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug