Bug 406028 - Folder right-click menu is missing items in the folder panel
Summary: Folder right-click menu is missing items in the folder panel
Status: RESOLVED WORKSFORME
Alias: None
Product: dolphin
Classification: Applications
Component: panels: folders (show other bugs)
Version: 18.12.3
Platform: Kubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords: junior-jobs, usability
Depends on:
Blocks:
 
Reported: 2019-03-30 00:40 UTC by Marcus Sundman
Modified: 2021-02-02 04:33 UTC (History)
10 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcus Sundman 2019-03-30 00:40:19 UTC
SUMMARY

Several items are missing in the folder's right-click popup-menu in the folder panel. Most notably "open in new tab", "open in new window", and "add to places".

STEPS TO REPRODUCE
1. Right-click a folder in the main panel.
2. Right-click a folder in the folder panel.

OBSERVED RESULT

The popup-menus contain completely different items.

EXPECTED RESULT

All right-click folder popup-menus to contain the same items, except for the very few panel-specific ones.

SOFTWARE/OS VERSIONS
KDE Plasma Version: 5.15.3
KDE Frameworks Version: 5.56.0
Qt Version: 5.11.3
Comment 1 Nate Graham 2019-04-03 21:09:39 UTC
Yep, looks like it. Probably not a hard patch to write, either. Wanna try your hand at it? I can help!
Comment 2 Abijith 2019-04-04 07:13:23 UTC
(In reply to Nate Graham from comment #1)
> Yep, looks like it. Probably not a hard patch to write, either. Wanna try
> your hand at it? I can help!

I am looking for assistance on fixing bugs, can you help me? I am very much interested in fixing but I have little trouble understanding stuff. I would like help.
Comment 3 Marcus Sundman 2019-04-04 23:38:33 UTC
(In reply to Abijith from comment #2)
> (In reply to Nate Graham from comment #1)
> > Yep, looks like it. Probably not a hard patch to write, either. Wanna try
> > your hand at it? I can help!
> 
> I am looking for assistance on fixing bugs, can you help me? I am very much
> interested in fixing but I have little trouble understanding stuff. I would
> like help.

When you find the relevant places in the code, please don't copy'n'paste the code that adds those menu entries, but have both places call the same piece of code. For example, you can have a function adding those menu entries, and then call the same function from both popup-menu-creation places. (That way, if someone adds more menu entries then those will automatically go to both menus.)
Comment 4 Nate Graham 2019-04-05 04:44:24 UTC
(In reply to Abijith from comment #2)
> (In reply to Nate Graham from comment #1)
> > Yep, looks like it. Probably not a hard patch to write, either. Wanna try
> > your hand at it? I can help!
> 
> I am looking for assistance on fixing bugs, can you help me? I am very much
> interested in fixing but I have little trouble understanding stuff. I would
> like help.
First get your development environment set up: https://community.kde.org/Get_Involved/development

Then compile Dolphin. After that, check out the code that implements the context menus. The easiest way to find it is to grep through the codebase for English string that appears in the context menu.


(In reply to Marcus Sundman from comment #3)
> When you find the relevant places in the code, please don't copy'n'paste the
> code that adds those menu entries, but have both places call the same piece
> of code. For example, you can have a function adding those menu entries, and
> then call the same function from both popup-menu-creation places. (That way,
> if someone adds more menu entries then those will automatically go to both
> menus.)
+1, at least for the menu items that are relevant to both the Places panel and the Folder panel.
Comment 5 Antonio Prcela 2019-07-07 21:20:06 UTC
Hi, I'm onto something here and would really like to solve this.
One thing tho: Where to put the function that would be called by both right click context menus. There are some header files that are used by both folderspane.cpp and solphinview.cpp , but none of the included headers makes much sense to be used (considering the name of the header file)

https://cgit.kde.org/dolphin.git/tree/src/panels/folders/folderspanel.cpp
https://cgit.kde.org/dolphin.git/tree/src/views/dolphinview.cpp
Comment 6 Marcus Sundman 2019-07-08 05:17:01 UTC
(In reply to Antonio from comment #5)
> Hi, I'm onto something here and would really like to solve this.
> One thing tho: Where to put the function that would be called by both right
> click context menus. There are some header files that are used by both
> folderspane.cpp and solphinview.cpp , but none of the included headers makes
> much sense to be used (considering the name of the header file)
> 
> https://cgit.kde.org/dolphin.git/tree/src/panels/folders/folderspanel.cpp
> https://cgit.kde.org/dolphin.git/tree/src/views/dolphinview.cpp

How about https://cgit.kde.org/dolphin.git/tree/src/dolphincontextmenu.cpp ?
Comment 7 Antonio Prcela 2019-07-08 15:28:21 UTC
(In reply to Marcus Sundman from comment #6)
> How about https://cgit.kde.org/dolphin.git/tree/src/dolphincontextmenu.cpp ?

Thanks, will add it there.
After that, treeviewcontextmenu might become unused:

https://cgit.kde.org/dolphin.git/tree/src/panels/folders/treeviewcontextmenu.h

As far as I can tell by using grep, it's only included in https://cgit.kde.org/dolphin.git/tree/src/panels/folders/folderspanel.cpp

I'm off and I hope to find some time in this week to fix it.
Comment 8 Nate Graham 2019-07-09 21:40:19 UTC
Great!
Comment 9 Antonio Prcela 2019-07-13 10:40:20 UTC
Hi, I finally understand the basics of how the context menu works in KDE.
So i used the signal 'requestContextMenu' when a user right clicks on a folder in the folder panel.
But I can't get around the problem that it always switches the open folder on the right side too (in the main view).

This code:
https://github.com/precla/dolphin/commit/8407835ec8c6f6fe6256cd60b8f66213ac0095ed

If I remove 'emit slotItemActivated(index);' (*1) and instead use 'updateCurrentItem(index);' (*2), then the context menu works on the currently open folder in the main view. See attached gifs.

Can anyone give me an hint or help me out?
Thanks in advance!

*1: https://i.imgur.com/SaM4JPv.gif
*2: https://i.imgur.com/8SaVwz6.gif
Comment 10 Nate Graham 2019-07-14 12:03:02 UTC
It's switching the view to show the right-clicked-on item because you're emitting slotItemActivated(), which does that.

How about if you emit neither slotItemActivated() nor updateCurrentItem()? If that doesn't work, then you may need to create a new one that's basically like slotItemActivated(), but doesn't activate the item.
Comment 11 Antonio Prcela 2019-07-15 20:17:34 UTC
(In reply to Nate Graham from comment #10)
> It's switching the view to show the right-clicked-on item because you're
> emitting slotItemActivated(), which does that.
> 
> How about if you emit neither slotItemActivated() nor updateCurrentItem()?
> If that doesn't work, then you may need to create a new one that's basically
> like slotItemActivated(), but doesn't activate the item.

Thanks for your input. Will try it!
Comment 12 Antonio Prcela 2019-07-22 20:17:19 UTC
(In reply to Nate Graham from comment #10)
> It's switching the view to show the right-clicked-on item because you're
> emitting slotItemActivated(), which does that.
> 
> How about if you emit neither slotItemActivated() nor updateCurrentItem()?
> If that doesn't work, then you may need to create a new one that's basically
> like slotItemActivated(), but doesn't activate the item.

unfortunately , I can't come up with another version of slotItemActivated() that would do this.
Problem, for me, so far is following:

To not copy-paste code and instead use the same function call for both the folder panel and the main view, I'd say that quite a bit would have to be rewritten/added.

The current function (method) that is being called when a right click is made, is the following:
https://cgit.kde.org/dolphin.git/tree/src/dolphinmainwindow.cpp#n918

Now, at line 923:
'new DolphinContextMenu(this, pos, item, url);'
Where 'this' is a 'DolphinMainWindow' object.

The url is passed properly, but since 'this' is used, it takes the property of the currently active window.

Whereas the 'this' that is sent via:
https://cgit.kde.org/dolphin.git/tree/src/panels/folders/folderspanel.cpp#n219
to:
https://cgit.kde.org/dolphin.git/tree/src/panels/folders/treeviewcontextmenu.h#n46
is of type 'FoldersPanel'.
So I just can't redirect it to 'new DolphinContextMenu()'. Either I overload some stuff from DolphinContextMenu, which might be a overkill or I keep my mind busy with this one until I come up with something simple and elegant.

/me ends monologue :)
Comment 13 Aldrin Tadas 2020-07-03 01:31:55 UTC
(In reply to Marcus Sundman from comment #3)
> (In reply to Abijith from comment #2)
> > (In reply to Nate Graham from comment #1)
> > > Yep, looks like it. Probably not a hard patch to write, either. Wanna try
> > > your hand at it? I can help!
> > 
> > I am looking for assistance on fixing bugs, can you help me? I am very much
> > interested in fixing but I have little trouble understanding stuff. I would
> > like help.
> 
> When you find the relevant places in the code, please don't copy'n'paste the
> code that adds those menu entries, but have both places call the same piece
> of code. For example, you can have a function adding those menu entries, and
> then call the same function from both popup-menu-creation places. (That way,
> if someone adds more menu entries then those will automatically go to both
> menus.)

Hello, seems like Places panel, Folders panel and the Main View have their own implementation of context menus. It would be impossible to add the missing "openInNewTab"/"openInNewWindow" functionality without rewriting one that is specific to the Folders Panel.

I want to start working on this one, but I'm gonna need guidance on where to start tackling this.
Comment 14 Bug Janitor Service 2020-07-13 15:34:31 UTC
A possibly relevant merge request was started @ https://invent.kde.org/system/dolphin/-/merge_requests/37
Comment 15 Justin Zobel 2021-01-03 03:31:33 UTC
(In reply to Aldrin Tadas from comment #13)
> (In reply to Marcus Sundman from comment #3)
> > (In reply to Abijith from comment #2)
> > > (In reply to Nate Graham from comment #1)
> > > > Yep, looks like it. Probably not a hard patch to write, either. Wanna try
> > > > your hand at it? I can help!
> > > 
> > > I am looking for assistance on fixing bugs, can you help me? I am very much
> > > interested in fixing but I have little trouble understanding stuff. I would
> > > like help.
> > 
> > When you find the relevant places in the code, please don't copy'n'paste the
> > code that adds those menu entries, but have both places call the same piece
> > of code. For example, you can have a function adding those menu entries, and
> > then call the same function from both popup-menu-creation places. (That way,
> > if someone adds more menu entries then those will automatically go to both
> > menus.)
> 
> Hello, seems like Places panel, Folders panel and the Main View have their
> own implementation of context menus. It would be impossible to add the
> missing "openInNewTab"/"openInNewWindow" functionality without rewriting one
> that is specific to the Folders Panel.
> 
> I want to start working on this one, but I'm gonna need guidance on where to
> start tackling this.

I see the MR was closed. Are you still working on this offline?
Comment 16 Bug Janitor Service 2021-01-18 04:33:15 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least
15 days. Please provide the requested information as soon as
possible and set the bug status as REPORTED. Due to regular bug
tracker maintenance, if the bug is still in NEEDSINFO status with
no change in 30 days the bug will be closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please
mark the bug as REPORTED so that the KDE team knows that the bug is
ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!
Comment 17 Bug Janitor Service 2021-02-02 04:33:16 UTC
This bug has been in NEEDSINFO status with no change for at least
30 days. The bug is now closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

Thank you for helping us make KDE software even better for everyone!