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

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

Reply via email to