RESOLVED FIXED 30336
[Qt] Make context menu to work in QGraphicsWebView 
https://bugs.webkit.org/show_bug.cgi?id=30336
Summary [Qt] Make context menu to work in QGraphicsWebView
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
DetailsFormatted DiffDiff
patch 0.2 - same as 0.1, but not leaky (2.26 KB, patch)
2009-10-13 09:44 PDT, Antonio Gomes 		hausmann: review-
DetailsFormatted DiffDiff
landed in r49618 - patch 0.3 (2.18 KB, patch)
2009-10-13 18:30 PDT, Antonio Gomes 		no flags
DetailsFormatted DiffDiff
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.