[webkit-reviews] review requested: [Bug 30336] [Qt] Make context menu to work in QGraphicsWebView : [Attachment 41144] patch 0.3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 13 18:30:59 PDT 2009


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

Attachment 41144: patch 0.3
https://bugs.webkit.org/attachment.cgi?id=41144&action=review

------- Additional Comments from Antonio Gomes (tonikitoo)
<tonikitoo at webkit.org>
> 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.


More information about the webkit-reviews mailing list