[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 120794: Implement Unique Mode Properly in Rekonq
From: David_Narváez <david.narvaez () computer ! org>
Date: 2014-11-02 17:23:18
Message-ID: 20141102172318.9636.17316 () probe ! kde ! org
[Download RAW message or body]
--===============3018501629002076160==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120794/
-----------------------------------------------------------
(Updated Nov. 2, 2014, 5:23 p.m.)
Status
------
This change has been marked as submitted.
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
--===============3018501629002076160==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120794/">https://git.reviewboard.kde.org/r/120794/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as \
submitted.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for KDE Frameworks and rekonq.</div>
<div>By David Narváez.</div>
<p style="color: grey;"><i>Updated Nov. 2, 2014, 5:23 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
rekonq
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
2em;line-height: inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;">Open one Rekonq \
window</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Try opening Rekonq again</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Try \
opeining Rekonq from a command line with some URLs</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">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)</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Open rekonq from the console using rekonq \
--incognito</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Open rekonq from the console using reknoq --webapp \
twitter.com</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Open rekonq from the console pointing it to some local \
HTML file</li> </ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/application.h <span style="color: grey">(7ccd60d)</span></li>
<li>src/application.cpp <span style="color: grey">(c7c297d)</span></li>
<li>src/main.cpp <span style="color: grey">(7592f7a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120794/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============3018501629002076160==--
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic