Review Request 120794: Implement Unique Mode Properly in Rekonq

[David Narváez]

> On Oct. 28, 2014, 9:32 a.m., David Faure wrote:
> > src/application.cpp, line 789
> > <https://git.reviewboard.kde.org/r/120794/diff/2/?file=322403#file322403line789>
> >
> >     Note that you might want to use the cwd in order to resolve urls with it, so that "rekonq localfile.html" works.
> >     This requires QUrl::fromUserInput(two args) from Qt 5.4 though, so probably too early to do right now, but you can at least add a comment, if not a #if QT_VERSION check.

Local URL handling is indeed broken, I added a TODO comment to work on it later, but added parameters that would be missing if I wanted to implement this properly in the handleCmdLine method


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120794/#review69243
-----------------------------------------------------------


On Oct. 28, 2014, 2 p.m., David Narváez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120794/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2014, 2 p.m.)
> 
> 
> Review request for KDE Frameworks and rekonq.
> 
> 
> Repository: rekonq
> 
> 
> Description
> -------
> 
> This is my humble attempt to implement the Unique Mode properly. I have been trying to do this for the longest time in a way that avoids code duplication but I can't find a way to jump over all the hurdles these API impose. I tried learning from other ports from KUniqueApplication but a quick look at LXR shows there are plenty of applications that blindly ported to Unique Mode but didn't bother implementing activateRequested and the one I found that did was plasmawindowedcorona.cpp which does not need a QCommandLineParser, so the code duplication is less evident. At this point, I would like someone who knows about the QCommandLineParser + KDBusAddons dance to look at this and tell if it is reasonable or not.
> 
> The current patch just makes it possible to open several Rekonq applications. It does not do the right thing when a Rekonq window is already open in the current activity and a user clicks a link elsewhere (step 4 in the Testing Done section) because it starts a brand new Rekonq window, but that's a different patch. It also does some funky thing asking you if you want to restore the previous session when nothing has crashed, I have to check that.
> 
> 
> Diffs
> -----
> 
>   src/application.h 7ccd60d 
>   src/application.cpp c7c297d 
>   src/main.cpp 7592f7a 
> 
> Diff: https://git.reviewboard.kde.org/r/120794/diff/
> 
> 
> Testing
> -------
> 
> 1. Open one Rekonq window
> 2. Try opening Rekonq again
> 3. Try opeining Rekonq from a command line with some URLs
> 4. Assuming Reknoq is your default browser (why wouldn't it be?) click on a link somewhere (I click on the links at the title of the Konversation channels I am in)
> 5. Open rekonq from the console using rekonq --incognito
> 6. Open rekonq from the console using reknoq --webapp twitter.com
> 7. Open rekonq from the console pointing it to some local HTML file
> 
> Before this patch, nothing happens in steps 2 - 7. After a first version of this patch that does not avoid the QCommandLine parser if the argument list is not empty, the window opened at 1 crashes because the activateRequested signal passes an empty list of arguments - not even the binary name - so QCommandLine parser dies. With this patch, every step opens a new window properly, step 5 opens an incognito window and step 6 opens a webapp window (simple window). Step 7 opens the current working directory because local URL handling is broken.
> 
> 
> Thanks,
> 
> David Narváez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141028/f78d6082/attachment.html>