> 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
> 
>

_______________________________________________
rekonq mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/rekonq

Reply via email to