Bug 45539

Summary: [Qt] selected text gets deleted when qgraphicswebview losts focus
Product: WebKit Reporter: Yi Shen <max.hong.shen>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ademar, commit-queue, robert, suresh.voruganti
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
proposal fix
kling: review-
patch with test case none

Description Yi Shen 2010-09-10 07:40:39 PDT
Precondition: using qgraphicswebview

Steps to reproduce:
1. open a page that has a textarea field, and type some characters in that field  (e.g.  I typed "abcdefg" in the textarea)
2. use your mouse to select (highlight) some words (characters) (e.g. I selected "defg")
3. click on other qgraphics ui component (e.g. url bar) which removes the focus from qgraphicswebview

Actual Result:
All the selected/highlight words (characters) get deleted (e.g. the "defg" get deleted and only "abcd" left in the textarea)

Reason:
When focus changed, qgraphicssence sends out an inputmethodevent with empty preedit string and empty QInputMethodEvent::Attribute to qgrapchiswebview, which finally be delivered to the QWebPage::inputMethodEvent(QInputMethodEvent *ev). And eventually, an empty preedit string is passed to WebCore::Editor::setComposition(...) which deletes all the selected text.

Fix proposal:
Check the preedit string before passing it to the WebCore::Editor::setComposition.
Comment 1 Yi Shen 2010-09-10 08:01:22 PDT
Created attachment 67183 [details]
proposal fix
Comment 2 Andreas Kling 2010-09-10 09:01:53 PDT
Comment on attachment 67183 [details]
proposal fix

> +            if (!ev->preeditString().isEmpty())

Should be "if (!preedit.isEmpty())"

Change looks reasonable, but it needs an autotest (see WebKit/qt/tests/)
Comment 3 Yi Shen 2010-09-10 09:05:10 PDT
yes, you are right. Thanks for reviewing, I will change it.

(In reply to comment #2)
> (From update of attachment 67183 [details])
> > +            if (!ev->preeditString().isEmpty())
> 
> Should be "if (!preedit.isEmpty())"
> 
> Change looks reasonable, but it needs an autotest (see WebKit/qt/tests/)
Comment 4 Yi Shen 2010-09-10 14:15:50 PDT
Created attachment 67236 [details]
patch with test case
Comment 5 WebKit Commit Bot 2010-09-11 00:07:33 PDT
Comment on attachment 67236 [details]
patch with test case

Clearing flags on attachment: 67236

Committed r67289: <http://trac.webkit.org/changeset/67289>
Comment 6 WebKit Commit Bot 2010-09-11 00:07:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Suresh Voruganti 2010-09-16 09:43:32 PDT
Ademar, can you cherry pick the fix for QtWebkit 2.1?

This fix is required for implementation of some features from browser team.
Comment 8 Ademar Reis 2010-09-16 10:20:21 PDT
Revision r67289 cherry-picked into qtwebkit-2.1 with commit 50c6993 <http://gitorious.org/webkit/qtwebkit/commit/50c6993>
Comment 9 Ademar Reis 2010-09-16 10:26:06 PDT
Actually last comment should be:

Revision r67289 cherry-picked into qtwebkit-2.1 with commit b422ce0 <http://gitorious.org/webkit/qtwebkit/commit/b422ce0>