[webkit-reviews] review denied: [Bug 30336] [Qt] Make context menu to work in QGraphicsWebView : [Attachment 41110] patch 0.2 - same as 0.1, but not leaky

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Antonio Gomes (tonikitoo)
<tonikitoo at webkit.org>'s request for review:
Bug 30336: [Qt] Make context menu to work in QGraphicsWebView
https://bugs.webkit.org/show_bug.cgi?id=30336

Attachment 41110: patch 0.2 - same as 0.1, but not leaky
https://bugs.webkit.org/attachment.cgi?id=41110&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>

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.