Bug 316322 - Open links in new tab only if there is a window in the current activity/on the current desktop
Summary: Open links in new tab only if there is a window in the current activity/on th...
Status: RESOLVED FIXED
Alias: None
Product: rekonq
Classification: Unclassified
Component: general (show other bugs)
Version: 2.2
Platform: Ubuntu Packages Linux
: NOR wishlist (vote)
Target Milestone: ---
Assignee: Andrea Diamantini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-07 16:00 UTC by Jonathan Verner
Modified: 2013-03-14 13:10 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch implementing the feature (7.64 KB, patch)
2013-03-07 18:24 UTC, Jonathan Verner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Verner 2013-03-07 16:00:41 UTC
I have the "open links in new tab" option set. However, when the current activity does not have a rekonq window open while a different activity has rekonq open, opening an url (e.g. via krunner) opens it in a window in a different activity. So to see the window, one must first switch activities and this disrupts the workflow.


Reproducible: Always

Steps to Reproduce:
1. Run Rekonq
2. Start a new empty activity
3. Use krunner to open an url
Actual Results:  
The url opens in the rekonq window which lives on the first activity


Expected Results:  
A new rekonq window is opened and associated with the current activity

I have been looking at the code in application.cpp: Application:tabWindow()
trying to change it to something like:

        KActivities::Consumer activityConsumer;
        QString currentActivity = activityConsumer.currentActivity()
        TabWindow *activityWindow = 0
        Q_FOREACH(const QWeakPointer<TabWindow> &pointer, m_tabWindows)
        {
            if (KWindowInfo(pointer.data()->effectiveWinId(), NET::WMDesktop, 0).isOnCurrentDesktop())
                return pointer.data();
            else if (isOnCurrentActivity(pointer.data()))
                activityWindow = pointer.data()
        }

         return activityWindow;

However I couldn't figure out how to write the isOnCurrentActivity function,
i.e. how to determine the activity that a window is associated to. (And I am not
sure that it would even work, and be efficient, the consumer is supposed to be
long-lived). After thinking about the problem I thought that maybe it would make 
sense to open a new window whenever an existing window is not present on the
current DESKTOP --- presumably, when I open an url, I want it to show up on
the current DESKTOP, not somewhere, where I need to switch to it first.
If I understand the code this would be achieved by just changing 
the line (again in Application::tabWindow())

return m_tabWindows.at(0).data();

to 

return 0;
Comment 1 Jonathan Verner 2013-03-07 16:41:52 UTC
Hmm, 

> I have been looking at the code in application.cpp: Application:tabWindow()
> trying to change it to something like:

Hmm, I see now that this is not the right place to change it :-)

> After thinking about the problem I thought that maybe it would
> make  sense to open a new window whenever an existing window is not 
> present on the current DESKTOP --- presumably, when I open an url, I want it to 
> show up on the current DESKTOP, not somewhere, where I need to switch to it 
> first. If I understand the code this would be achieved by just changing 
> the line (again in Application::tabWindow())

Again, my reading of the code was wrong. However even if it was changed to work, the solution with opening a new window if the current desktop does not have an open window is not very satisfactory... I will try to see if I can come up with a different solution...



> 
> return m_tabWindows.at(0).data();
> 
> to 
> 
> return 0;
Comment 2 Jonathan Verner 2013-03-07 18:24:07 UTC
Created attachment 77839 [details]
Patch implementing the feature

O.k., so I tried implementing a "proper" solution and the result is this patch. Should work, but I am not sure whether the coding style is o.k. and whether I haven't introduced any bugs by misunderstanding what some methods do.
Comment 3 Andrea Diamantini 2013-03-12 17:31:37 UTC
Hi Jonathan, many thanks for your work about and sorry for my late reply. I'm going to evaluate your code which seems good, but I'm not convinced a lot from this "Activities" affair. It seems to me a real strange thing I have (inside my application) to take care of it. I'd just like to call "show()" and that's all.
Comment 4 Andrea Diamantini 2013-03-12 17:31:53 UTC
Hi Jonathan, many thanks for your work about and sorry for my late reply. I'm going to evaluate your code which seems good, but I'm not convinced a lot from this "Activities" affair. It seems to me a real strange thing I have (inside my application) to take care of it. I'd just like to call "show()" and that's all.
Comment 5 Jonathan Verner 2013-03-12 21:21:32 UTC
(In reply to comment #4)
> Hi Jonathan, many thanks for your work about and sorry for my late reply.

No problem, five days seem pretty promt to me :-)

> I'm going to evaluate your code which seems good, but I'm not convinced a
> lot from this "Activities" affair.  It seems to me a real strange thing I
> have (inside my application) to take care of it. I'd just like to call
> "show()" and that's all.

To me, the "Activities" thing sounded promising, so I wanted to give it a try.
This "bug" was something, which made them basically unuseable for me, so
I went ahead and tried to fix it. However I can completely understand, if you
reject the "fix" --- I can see how it could be regarded as an unnecessary maintenance burden. Also, maybe I should have tried to talk with the "activities" developers and ask their opinion about solving this problem. It's just that I don't have much time and I didn't want to waste their time when I wasn't sure I could commit to working on this.  I do hope I haven't wasted your time instead!

Btw. thanks for your work, I like rekonq very much!!
Comment 6 Andrea Diamantini 2013-03-13 15:19:04 UTC
No, you did the right choice working on rekonq because on the Actual
"Activities Design" every app should take care of this. The same is true
for the new activity manager and *I'm sure the same will be true for their
next astonishing piece of software.*
*What can I add about? Just the impression that they are pushing
integration a bit over. I claim my calm "black" desktop :) *


2013/3/12 Jonathan Verner <jonathan.verner@gmail.com>

> https://bugs.kde.org/show_bug.cgi?id=316322
>
> --- Comment #5 from Jonathan Verner <jonathan.verner@gmail.com> ---
> (In reply to comment #4)
> > Hi Jonathan, many thanks for your work about and sorry for my late reply.
>
> No problem, five days seem pretty promt to me :-)
>
> > I'm going to evaluate your code which seems good, but I'm not convinced a
> > lot from this "Activities" affair.  It seems to me a real strange thing I
> > have (inside my application) to take care of it. I'd just like to call
> > "show()" and that's all.
>
> To me, the "Activities" thing sounded promising, so I wanted to give it a
> try.
> This "bug" was something, which made them basically unuseable for me, so
> I went ahead and tried to fix it. However I can completely understand, if
> you
> reject the "fix" --- I can see how it could be regarded as an unnecessary
> maintenance burden. Also, maybe I should have tried to talk with the
> "activities" developers and ask their opinion about solving this problem.
> It's
> just that I don't have much time and I didn't want to waste their time
> when I
> wasn't sure I could commit to working on this.  I do hope I haven't wasted
> your
> time instead!
>
> Btw. thanks for your work, I like rekonq very much!!
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
>
Comment 7 Andrea Diamantini 2013-03-13 17:59:06 UTC
Git commit a34f7ad76fe02227cc59143d91dabb891c2e3605 by Andrea Diamantini.
Committed on 13/03/2013 at 17:50.
Pushed by adjam into branch 'ActivitiesSupport'.

Support Activities

Open links in new tab only if there is a window
in the current activity/on the current desktop

This code has been written by Jonathan Verner and reviewed (a bit)
by me. Hope everyone will be happy now...
REVIEWED-BY: adjam

M  +17   -0    CMakeLists.txt
A  +1    -0    config-kactivities.h.cmake
M  +7    -0    src/CMakeLists.txt
M  +93   -37   src/application.cpp
M  +34   -1    src/application.h

http://commits.kde.org/rekonq/a34f7ad76fe02227cc59143d91dabb891c2e3605
Comment 8 Andrea Diamantini 2013-03-13 18:00:22 UTC
Jonathan,
I need your help to test this code. Can you please try the "ActivitiesSupport" branch and let me know if it really works?
Comment 9 Jonathan Verner 2013-03-14 11:35:37 UTC
Checked it out, and it works! Thanks!

(In reply to comment #8)
> Jonathan,
> I need your help to test this code. Can you please try the
> "ActivitiesSupport" branch and let me know if it really works?
Comment 10 Andrea Diamantini 2013-03-14 13:10:46 UTC
No, thank YOU!
We'll have it in master in a while. Thanks again!


2013/3/14 Jonathan Verner <jonathan.verner@gmail.com>

> https://bugs.kde.org/show_bug.cgi?id=316322
>
> --- Comment #9 from Jonathan Verner <jonathan.verner@gmail.com> ---
> Checked it out, and it works! Thanks!
>
> (In reply to comment #8)
> > Jonathan,
> > I need your help to test this code. Can you please try the
> > "ActivitiesSupport" branch and let me know if it really works?
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
>