Bug 39019 - [Qt] Combobox doesn't close when scrolling in graphicsbased mode
Summary: [Qt] Combobox doesn't close when scrolling in graphicsbased mode
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Major
Assignee: Jocelyn Turcotte
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 35784
  Show dependency treegraph
 
Reported: 2010-05-12 13:05 PDT by Kenneth Rohde Christiansen
Modified: 2010-08-20 15:12 PDT (History)
11 users (show)

See Also:


Attachments
Screenshot showing the issue (99.64 KB, image/png)
2010-05-12 13:05 PDT, Kenneth Rohde Christiansen
no flags Details
Patch (4.62 KB, patch)
2010-06-29 03:23 PDT, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
Patch v2 (4.77 KB, patch)
2010-06-29 04:09 PDT, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
Patch (complementary to the one in Qt) (2.87 KB, patch)
2010-06-30 10:29 PDT, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2010-05-12 13:05:17 PDT
Created attachment 55890 [details]
Screenshot showing the issue

The combobox in graphics mode is using a proxy widget and thus doesn't seem to be notified when the proxy widget looses focus, resulting in wrong rendering.
Comment 1 Simon Hausmann 2010-05-18 14:19:08 PDT
Sounds like a Qt (QGraphicsView) bug...

Can you try to make a Qt-only testcase for it and report it upstream?
Comment 2 Simon Hausmann 2010-05-29 03:17:45 PDT
Alexis, have you seen something like this before? Could it be a QGraphicsProxyWidget problem?
Comment 3 Viatcheslav Ostapenko 2010-06-02 17:28:45 PDT
I think we have similar bug, but not exactly the same: https://bugs.webkit.org/show_bug.cgi?id=40082
Comment 4 Jocelyn Turcotte 2010-06-29 03:23:54 PDT
Created attachment 59999 [details]
Patch
Comment 5 WebKit Review Bot 2010-06-29 03:27:55 PDT
Attachment 59999 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKit/qt/WebCoreSupport/QtFallbackWebPopup.cpp:110:  Declaration has space between type name and * in QGraphicsView *firstView  [whitespace/declaration] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Jocelyn Turcotte 2010-06-29 04:09:25 PDT
Created attachment 60003 [details]
Patch v2

Fixed style issues.
Comment 7 Kenneth Rohde Christiansen 2010-06-29 05:53:50 PDT
Comment on attachment 60003 [details]
Patch v2

Fine with me. Girish made the original patch because it was needed for SecondLife, maybe you should notify him.
Comment 8 Antonio Gomes 2010-06-29 07:58:10 PDT
(In reply to comment #6)
> Created an attachment (id=60003) [details]
> Patch v2
> 
> Fixed style issues.

does you patch fix bug 39066?
Comment 9 Antonio Gomes 2010-06-29 07:58:41 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > Created an attachment (id=60003) [details] [details]
> > Patch v2
> > 
> > Fixed style issues.
> 
> does you patch fix bug 39066?

/s/you/your
Comment 10 Jocelyn Turcotte 2010-06-29 08:09:13 PDT
(In reply to comment #8)
> (In reply to comment #6)
> > Created an attachment (id=60003) [details] [details]
> > Patch v2
> > 
> > Fixed style issues.
> 
> does you patch fix bug 39066?

No but I had a similar problem which this patch corrects. The context menu bug feels like there is a graphicsView->mapFromScene(graphicsWebView->mapToScene(pos)) missing somewhere.
Comment 11 Jocelyn Turcotte 2010-06-29 08:15:15 PDT
Comment on attachment 60003 [details]
Patch v2

I think that this patch should not go in.

After having a look I saw that QtAbstractWebPopup is not exposed in the API, and this patch would change the containment of a QGraphicsWebView inside a scene like SecondLife probably needed.

To use a QWidget combo box popup we would need to know if the developer wants to use QGraphicsWebView for its advantages (like accelerated compositing, tiling) or because he wants it inside a QGraphicsScene. Until we have a flag or a class that separate these two cases I think we should keep this behavior for popups.

I have another fix (mostly in Qt) for this bug and I'll push it when possible.
Comment 12 Jocelyn Turcotte 2010-06-30 10:29:59 PDT
Created attachment 60130 [details]
Patch (complementary to the one in Qt)

The patch in Qt will close the popop on wheel event like for QWidgets outside a graphic scene.

This patch make sure that we catch the popup hide event since the popup is directly closed by the QGraphicsScene.
Comment 13 WebKit Commit Bot 2010-06-30 11:49:46 PDT
Comment on attachment 60130 [details]
Patch (complementary to the one in Qt)

Clearing flags on attachment: 60130

Committed r62193: <http://trac.webkit.org/changeset/62193>
Comment 14 WebKit Commit Bot 2010-06-30 11:49:55 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Simon Hausmann 2010-07-01 01:26:53 PDT
Revision r62193 cherry-picked into qtwebkit-2.0 with commit 4bb61fb7b128d99bb98c410716a19e83cdeffe85
Comment 16 Girish Ramakrishnan 2010-08-20 15:12:50 PDT
I realize this bug is closed but the reason to use a proxy widget is so that the scene can render the popup when using QGraphicsScene::render(). With a QWidget combo box, the popup simply won't render since it's not part of the scene.