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

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


Still not very convinced by the behavior being defined in the event handler directly. otherwise it seems fine.


src/webview.cpp
<http://git.reviewboard.kde.org/r/100201/#comment431>

    here in the mousePressEvent event handler, it would be cleaner to emit a signal like linkCtrlClicked(const KUrl &), splitting the existing signal into two new ones (well it's only one if you remove the old signal).
    
    Then connect your slots to the appropriate signals.
    You could get away with a single slot that takes the enum value corresponding to Rekonq::NewTab (I don't have the source open at hand), and add an extra signal to connect linkCtrlClicked and linkMiddleClicked to, that would add this value, then connect this intermediate signal to your slot.
    
    Erm, not sure if that's clear



src/webview.cpp
<http://git.reviewboard.kde.org/r/100201/#comment429>

    you probably want to check the button:
    http://doc.trolltech.com/latest/qmouseevent.html#button
    
    this way you don't need the comment, and it's more robust in case things change in the future.
    
    emit linkMiddleClicked(result.linkUrl()); 
    could then go here (or something in that fashion)


- Pierre


On 2010-12-13 13:02:25, Furkan Üzümcü wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100201/
> -----------------------------------------------------------
> 
> (Updated 2010-12-13 13:02:25)
> 
> 
> 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/20101216/f57b2f67/attachment-0001.htm