Bug 30336

Summary:

[Qt] Make context menu to work in QGraphicsWebView

Product:

WebKit

Reporter:

Antonio Gomes <tonikitoo>

Component:

WebKit Qt

Assignee:

Antonio Gomes <tonikitoo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

hausmann

Priority:

Keywords:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
patch 0.1	none
patch 0.2 - same as 0.1, but not leaky	hausmann: review-
landed in r49618 - patch 0.3	none

Antonio Gomes

Reported 2009-10-13 09:24:43 PDT

currently csm are not working in qgwv ...

Attachments
patch 0.1 (2.27 KB, patch) 2009-10-13 09:27 PDT, Antonio Gomes	no flags	Details Formatted Diff Diff
patch 0.2 - same as 0.1, but not leaky (2.26 KB, patch) 2009-10-13 09:44 PDT, Antonio Gomes	hausmann: review-	Details Formatted Diff Diff
landed in r49618 - patch 0.3 (2.18 KB, patch) 2009-10-13 18:30 PDT, Antonio Gomes	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Antonio Gomes

Comment 1 2009-10-13 09:27:59 PDT

Created attachment 41106 [details] patch 0.1

Antonio Gomes

Comment 2 2009-10-13 09:44:59 PDT

Created attachment 41110 [details] patch 0.2 - same as 0.1, but not leaky

Simon Hausmann

Comment 3 2009-10-13 15:19:38 PDT

Comment on attachment 41110 [details] patch 0.2 - same as 0.1, but not leaky Argh, I'm sorry Antonio, I overlooked two details about the patch :-/ > + if (QGraphicsSceneContextMenuEvent* ev = static_cast<QGraphicsSceneContextMenuEvent*>(event)) { This if (and scope) isn't necessary, as the cast can't fail and as event is guaranteed to be non-zero. > + QContextMenuEvent fakeEvent(QContextMenuEvent(QContextMenuEvent::Mouse, ev->pos().toPoint())); It looks like there's a redundant nesting :), i.e. the could should probably read QContextMenuEvent fakeEvent(reason, ev->pos().toPoint()); instead of QContextMenuEvent fakeEvent(QContextMenuEvent(...)); I also think we should map ev->reason() to the QContextMenuEvent::Reason. Sorry :-/, I should've spotted this on the first review.

Antonio Gomes

Comment 4 2009-10-13 18:30:59 PDT

Created attachment 41144 [details] landed in r49618 - patch 0.3 > Argh, I'm sorry Antonio, I overlooked two details about the patch :-/ > I should've spotted this on the first review. thx again for re-reviewing, simon. > > + if (QGraphicsSceneContextMenuEvent* ev = static_cast<QGraphicsSceneContextMenuEvent*>(event)) { > This if (and scope) isn't necessary, as the cast can't fail and as event is > guaranteed to be non-zero. totally agree. done ... > > + QContextMenuEvent fakeEvent(QContextMenuEvent(QContextMenuEvent::Mouse, ev->pos().toPoint())); > It looks like there's a redundant nesting :), i.e. the could should probably > read errr ... likely copy and paste error. fixed. > I also think we should map ev->reason() to the QContextMenuEvent::Reason. true. also done.

Simon Hausmann

Comment 5 2009-10-13 22:05:13 PDT

Comment on attachment 41144 [details] landed in r49618 - patch 0.3 Thanks!

Antonio Gomes

Comment 6 2009-10-15 05:43:00 PDT

landed in r49618

Antonio Gomes

Comment 7 2009-10-15 10:17:59 PDT

Comment on attachment 41144 [details] landed in r49618 - patch 0.3 clearing r+ flag since patch has landed.

Antonio Gomes

Comment 8 2009-10-21 20:10:32 PDT

*** Bug 29180 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.