[rekonq] Re: Review Request: Added shortcuts for vi-like scrolling using h/l/j/k keys

[Pierre Rossi]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101782/#review4213
-----------------------------------------------------------


I'm a bit surprised this even works, event1 goes out of scope before even calling the parent implementation, which means your event points to some random memory (possibly the local variables for KWebView::keyPressEvent) at the top of the stack. Doesn't look right to me.




src/webview.cpp
<http://git.reviewboard.kde.org/r/101782/#comment3465>

    This should really be event = new QKeyEvent...
    Also you probably want to accept the existing one before you do that, so that it doesn't get propagated.



src/webview.cpp
<http://git.reviewboard.kde.org/r/101782/#comment3466>

    else if for that one and the next ones maybe ?
    


- Pierre


On June 27, 2011, 12:39 a.m., Thomas Murach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101782/
> -----------------------------------------------------------
> 
> (Updated June 27, 2011, 12:39 a.m.)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> This patch adds shortcuts for scrolling as in vi or konqueror and meets bug #238761.
> Things reviewers could check:
>  - is it wanted to not scroll when shift (or ctrl or other modifiers) is pressed? If so, I could produce another patch for usual scrolling using arrow keys + modifiers.
>  - would you propose using KActions?
> 
> 
> Diffs
> -----
> 
>   src/webview.cpp ca7f1b7 
> 
> Diff: http://git.reviewboard.kde.org/r/101782/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20110627/f5c4ddbd/attachment.htm