Bug 405620

Summary:	Move 'Safely Remove' down in Places popup menu
Product:	[Applications] dolphin	Reporter:	Christoph Feck <cfeck>
Component:	panels: places	Assignee:	Dolphin Bug Assignee <dolphin-bugs-null>
Status:	RESOLVED FIXED
Severity:	wishlist	CC:	mikhail.krutov, nate
Priority:	NOR	Keywords:	junior-jobs
Version:	unspecified
Target Milestone:	---
Platform:	Other
OS:	Linux
See Also:	https://bugs.kde.org/show_bug.cgi?id=409381
Latest Commit:	https://commits.kde.org/dolphin/1eaf5d3e0362f8d39655e973bcd29813ae98652b	Version Fixed In:
Sentry Crash Report:

Description Christoph Feck 2019-03-18 23:00:11 UTC

SUMMARY

Used to using multiple Dolphin windows, I usually right-click a icon in places panel and select "Open in New Window". Luckily, this is always the first entry, except for external devices, where (when they are unmounted), it is replaced by the "Mount" entry. This makes some sense, because when I want to open it, I maybe should mount it first (but maybe Dolphin could figure that out automatically; anyway, this is not what this wish is about).

The other case where "Open in New Window" is not the first entry is for mounted external devices. There it is replaced by the "Safely Remove". Considering that the first menu item can (and is) often unintendently exectuted (see https://bugreports.qt.io/browse/QTBUG-6632) but that bug is closed as "intended", we shouldn't have potentionally destructive operations as the first item in a popup menu. I suggest to move it down.

STEPS TO REPRODUCE
1. have an external mechanical disk mounted and powered
2. try to open it in a new window by quickly selecting "Open in New Window" in its popup menu
3. but accidentely move the mouse a bit towards the menu while clicking RMB, causing the menu to immediately close and execute its first item unintendently.

OBSERVED RESULT
cringe because you didn't expect the abrupt power down of the disk and fear that any uneeded power-up/-down cycle dimunishes the lifetime of the disk

EXPECTED RESULT
The "Open in New Window" is still the first item, as it is with all other places icons.

SOFTWARE/OS VERSIONS
Dolphin master, KF5 master, Qt 5.12.2

Comment 1 Christoph Feck 2019-03-18 23:01:48 UTC

Because of bug 405617 I cannot even re-mount the disk without disconnecting and reconnecting it again, which makes it even more "destructive".

Comment 2 Christoph Feck 2019-03-18 23:11:54 UTC

Actually, QTBUG-6632 is still open as https://bugreports.qt.io/browse/QTBUG-57849

Comment 3 Nate Graham 2019-03-18 23:42:40 UTC

I can understand where you're coming from. However I just want to point out that safely unmounting a disk is not destructive. :) Annoying if you do it by accident, maybe, but it's not destroying anything. :)

I'd be okay with a patch that moves the Unmount action down a few rows. Maybe move the separator to below "Open in New Tab" and put the Unomunt action under there. Wanna submit such a patch?

Comment 4 Christoph Feck 2019-03-21 00:49:13 UTC

That should be easy to do, https://cgit.kde.org/dolphin.git/tree/src/panels/places/placespanel.cpp#n161

Comment 5 Mike Krutov 2019-03-22 23:40:11 UTC

I've created a MR for this https://phabricator.kde.org/D19987

Please note, there are test failures on current master:

FAIL!  : PlacesItemModelTest::testDefaultViewProperties(Baloo - Documents) Compared values are not the same                                                                                  
   Loc: [/home/neko/src-kde/dolphin/src/tests/placesitemmodeltest.cpp(479)]                                                                                                                  
FAIL!  : PlacesItemModelTest::testDefaultViewProperties(Places - Audio) Compared values are not the same                                                                                     
   Loc: [/home/neko/src-kde/dolphin/src/tests/placesitemmodeltest.cpp(479)]               
FAIL!  : PlacesItemModelTest::testDefaultViewProperties(Baloo - Today) Compared values are not the same
   Loc: [/home/neko/src-kde/dolphin/src/tests/placesitemmodeltest.cpp(479)]                               

Those tests fail without my changes, and I'm not sure what change has introduced those failures.

Comment 6 Nate Graham 2019-03-24 17:41:50 UTC

Git commit 1eaf5d3e0362f8d39655e973bcd29813ae98652b by Nate Graham, on behalf of Tigran Gabrielyan.
Committed on 24/03/2019 at 17:31.
Pushed by ngraham into branch 'Applications/19.04'.

Move Safely Remove down in places context menu

Summary: BUG: 405620

Reviewers: #dolphin, ngraham, elvisangelaccio

Reviewed By: #dolphin, ngraham, elvisangelaccio

Subscribers: elvisangelaccio, ngraham, kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D19950

M  +16   -16   src/panels/places/placespanel.cpp

https://commits.kde.org/dolphin/1eaf5d3e0362f8d39655e973bcd29813ae98652b