[webkit-reviews] review denied: [Bug 28862] [Qt][API] Add a new QGraphicsWidget based version of the "QWebView" : [Attachment 39125] patch 2 - v0.1 - Add support for handling QGraphicsScene events.

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Antonio Gomes (tonikitoo)
<tonikitoo at gmail.com>'s request for review:
Bug 28862: [Qt][API] Add a new QGraphicsWidget based version of the "QWebView"
https://bugs.webkit.org/show_bug.cgi?id=28862

Attachment 39125: patch 2 - v0.1 - Add support for handling QGraphicsScene
events.
https://bugs.webkit.org/attachment.cgi?id=39125&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
In principle this approach is good, QWebPage _should_ become capable of dealing
with QGraphicsScene events.

However this implementation duplicates a some of the existing event handlers
(drag'n'drop) and it converts
the graphicsview events to QWidget events and then calls the original handlers.
This means a lot of unnecessary
object creation.

I think it would be better to change the existing handlers, for example
QWebPagePrivate::mousePressEvent, to
take a PlatformMouseEvent as argument. Then PlatformMouseEvent should be
extended with a constructor that
accepts a QGraphicsSceneMouseEvent. This way the code in QWebPage::event()
takes care of just calling the
right constructors, the PlatformFooEvent constructors take care of extracting
the parameters and the logic
in QWebPagePrivate's event handlers can remain without the need of duplication.