[webkit-reviews] review denied: [Bug 20303] [Qt] Key events are not working in frames. : [Attachment 30560] Oops! The previous patch was wrong.

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has denied Yongjun Zhang
<yongjun.zhang at nokia.com>'s request for review:
Bug 20303: [Qt] Key events are not working in frames.
https://bugs.webkit.org/show_bug.cgi?id=20303

Attachment 30560: Oops! The previous patch was wrong.
https://bugs.webkit.org/attachment.cgi?id=30560&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>

> -    if (!mainFrame->d->frame->eventHandler()->scrollOverflow(direction,
granularity))
> -	   mainFrame->d->frame->view()->scroll(direction, granularity);
> +    bool handled = true;
> +    if (!frame->eventHandler()->scrollOverflow(direction, granularity)) {
> +	   handled = frame->view()->scroll(direction, granularity);
> +	   Frame* parent = frame->tree()->parent();
> +	   while(!handled && parent) {
> +	       handled = parent->view()->scroll(direction, granularity);
> +	       parent = parent->tree()->parent();
> +	   }
> +    }

Are you sure you want to descent here? We are using focusedOrMainFrame() in the
qwebpage.cpp. I don't think it is appropriate to descent here...

I think you want the following code..

    if (!frame->eventHandler()->scrollOverflow(direction, granularity))
	return frame->view()->scroll(direction, granularity);
    return false;

you return true... could you elaborate?

and it would be great if you could create a LayoutTest for this bug. We have an
EventSender in Qt which allows you to send key events from HTML and then you
can check if the keys arrive at the right frame. Could you attempt doing that?