Bug 43169 - [Qt] Html autofocus not working with QGraphicsWebView
Summary: [Qt] Html autofocus not working with QGraphicsWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 50010
Blocks:
  Show dependency treegraph
 
Reported: 2010-07-28 21:19 PDT by Antonio Gomes
Modified: 2010-11-24 18:11 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.69 KB, patch)
2010-11-21 03:48 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff
Patch (5.69 KB, patch)
2010-11-22 13:05 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff
Patch (5.68 KB, patch)
2010-11-23 23:59 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff
Patch (1.32 KB, patch)
2010-11-24 14:37 PST, Jan Erik Hanssen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2010-07-28 21:19:23 PDT
Launch QtTestBrowser in graphics-based mode with

$ run-launcher --qt -graphicsbased http://google.com

the google page auto focus on the main textfield after it finishes loading the page. However, with graphicsbased QtTestBrowser it does not work, while works fine with QWidget based (QWebView).
Comment 1 Jan Erik Hanssen 2010-11-21 03:26:28 PST
(In reply to comment #0)
> Launch QtTestBrowser in graphics-based mode with
> 
> $ run-launcher --qt -graphicsbased http://google.com
> 
> the google page auto focus on the main textfield after it finishes loading the page. However, with graphicsbased QtTestBrowser it does not work, while works fine with QWidget based (QWebView).

The problem seems to be that QGraphicsScene only propagates focus events if they are caused by tab or backtab in cases where there is no active focus item. Only way around this that I can see is to install an event filter on the scene to capture the focus events.

I'll upload a possible patch shortly.
Comment 2 Jan Erik Hanssen 2010-11-21 03:48:21 PST
Created attachment 74498 [details]
Patch
Comment 3 Kenneth Rohde Christiansen 2010-11-22 00:36:25 PST
Comment on attachment 74498 [details]
Patch

Is it possible to make an autotest for this somehow?
Comment 4 Jan Erik Hanssen 2010-11-22 00:46:10 PST
(In reply to comment #3)
> (From update of attachment 74498 [details])
> Is it possible to make an autotest for this somehow?

Think so, should be possible to make a subclass of QGraphicsWebView and see if focusInEvent() is received after showing the view. I'll see if I can get a test up and running.
Comment 5 Jan Erik Hanssen 2010-11-22 13:05:20 PST
Created attachment 74588 [details]
Patch
Comment 6 Kenneth Rohde Christiansen 2010-11-23 01:13:52 PST
Comment on attachment 74588 [details]
Patch

Would be cool if you could do a test with multiple scenes.
Comment 7 Kenneth Rohde Christiansen 2010-11-23 01:14:52 PST
It would be really cool if you could make a similar patch for our WebKit2 port.
Comment 8 Jan Erik Hanssen 2010-11-23 02:48:25 PST
(In reply to comment #6)
> (From update of attachment 74588 [details])
> Would be cool if you could do a test with multiple scenes.

A test where you have a view visible and changing the scene you mean? Shouldn't be hard.
Comment 9 Jan Erik Hanssen 2010-11-23 03:18:28 PST
(In reply to comment #7)
> It would be really cool if you could make a similar patch for our WebKit2 port.

I'll see if I can find some information about how to start hacking on that.
Comment 10 Jan Erik Hanssen 2010-11-23 23:59:04 PST
Created attachment 74726 [details]
Patch
Comment 11 Kenneth Rohde Christiansen 2010-11-24 01:14:16 PST
Comment on attachment 74726 [details]
Patch

What is changed in this new patch? still looks good
Comment 12 WebKit Commit Bot 2010-11-24 01:35:15 PST
Comment on attachment 74726 [details]
Patch

Clearing flags on attachment: 74726

Committed r72650: <http://trac.webkit.org/changeset/72650>
Comment 13 WebKit Commit Bot 2010-11-24 01:35:20 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Simon Hausmann 2010-11-24 02:08:40 PST
Ouch, an event-filter on the scene. That sounds _really_ evil. IMHO we should roll out the patch.

I think the bug is actually in QtTestBrowser, in WebViewGraphicsBased::WebViewGraphicsBased, which should make the m_item the focus item.
Comment 15 Simon Hausmann 2010-11-24 03:36:49 PST
Rolled out with https://bugs.webkit.org/show_bug.cgi?id=50010
Comment 16 Jan Erik Hanssen 2010-11-24 14:37:22 PST
Created attachment 74794 [details]
Patch

Second attempt at fixing this, using the suggestion in #14
Comment 17 Antonio Gomes 2010-11-24 14:41:20 PST
Comment on attachment 74794 [details]
Patch

that was what I first suggest in bug description :)
Comment 18 Jan Erik Hanssen 2010-11-24 14:46:26 PST
(In reply to comment #17)
> (From update of attachment 74794 [details])
> that was what I first suggest in bug description :)

Indeed. My first solution was too complex for this.
Comment 19 WebKit Commit Bot 2010-11-24 16:41:35 PST
The commit-queue encountered the following flaky tests while processing attachment 74794 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html
svg/dom/SVGScriptElement/script-set-href.svg

Please file bugs against the tests.  These tests were authored by dumi@chromium.org, mitz@webkit.org, and zimmermann@kde.org.  The commit-queue is continuing to process your patch.
Comment 20 WebKit Commit Bot 2010-11-24 18:11:19 PST
Comment on attachment 74794 [details]
Patch

Clearing flags on attachment: 74794

Committed r72712: <http://trac.webkit.org/changeset/72712>
Comment 21 WebKit Commit Bot 2010-11-24 18:11:26 PST
All reviewed patches have been landed.  Closing bug.