[rekonq] Re: Review Request: Use Ctrl+Left Click to open a link in a new focused tab, even when the option "Open tabs in background" is enabled.

[Pierre Rossi]

> On 2010-12-13 11:24:02, Andrea Diamantini wrote:
> > Your patch works well :)
> > Anyway, from coding point of view, I think you can achieve the same result, just changing one line:
> > webview.cpp:618 --> emit loadUrl(url, Rekonq::NewFocusedTab) 
> > and adding one simple comment. If you prefer your implementation for some reasons, please add some comments in your code and remove the no more used loadUrlinNewTab slot.

+1 on the approach suggested by Andrea, please refrain from hijacking events, as it takes precedence over everything else and proved to provide headaches for people implementing stuff in the same areas after that. The design pattern consisting in emitting the signals needed from the event handlers and connecting to them seems more elegant to me, and the negligible overhead of a few function calls is something we can afford really.


- Pierre


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


On 2010-12-12 19:07:21, Furkan Üzümcü wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100201/
> -----------------------------------------------------------
> 
> (Updated 2010-12-12 19:07:21)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> Use Ctrl+Left Click to open a link in a new focused tab, even when the option "Open tabs in background" is enabled.
> 
> 
> Diffs
> -----
> 
>   src/webview.cpp e90b8da 
> 
> Diff: http://git.reviewboard.kde.org/r/100201/diff
> 
> 
> Testing
> -------
> 
> Tested by me and Panagiotis Papadopoulos and it works ok.
> 
> 
> Thanks,
> 
> Furkan
> 
>

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