[Places panel] Don't show Root by default
ClosedPublic

Authored by meven on Sep 25 2018, 2:27 AM.

Details

Summary

This patch removes Root from the default Places panel for new installations, because it's redundant; you can still get to / on your machine in one click using the appropriate disk entry on the bottom of the Places panel. The appropriate disk entry now has a unique icon to help you distinguish it from others (added in D16653).

By removing it, we gain room to add something more useful such as a Recently Used item (see D7446) without making the Places panel show a vertical scrollbar with Dolphin's default size.

Depends on D16653

Test Plan

Log in with a new user account or rm ~/.local/share/user-places.xbel and open Dolphin; notice that Root is not on the Places panel.

Existing user accounts are untouched.

Diff Detail

Repository
R241 KIO
Branch
arcpatch-D15739
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12790
Build 12808: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

All arguments make sense. It even looks a lot better without the red folder icon. +1

Do four +1s amount to any formal Accept statuses? ;-)

davidedmundson requested changes to this revision.Sep 25 2018, 4:02 PM
davidedmundson added a subscriber: davidedmundson.

They don't.

Why are you changing the if (windows) code path above the elif?

This revision now requires changes to proceed.Sep 25 2018, 4:02 PM

They don't.

Why are you changing the if (windows) code path above the elif?

Because that's the windows equivalent of adding Root. I reasoned that if we don't want to show a Places panel entry for Root on Linux, then for similar reasons we wouldn't want to create Places panel entries for Windows disks when on Windows. Don't disks show up in the Devices section on Windows like they do on Linux? Can't test, sorry.

No idea.

It isn't the same as adding root as it adds every drive quite deliberately.

There is a solid windows device back end, but it needs asking someone.

ngraham added a subscriber: bruns.Sep 25 2018, 5:06 PM

@bruns, do you know if Solid (and by extension, the Places panel) shows Windows disks when Dolphin is run on Windows?

Side note: does anyone run Dolphin on Windows? Do we distribute it at all?

bruns added a comment.Sep 25 2018, 8:38 PM

They don't.

Why are you changing the if (windows) code path above the elif?

Note this is _WIN32_WCE, i.e. dead code. WCE is no more supported by any half way recent Qt version.

bruns added a comment.Sep 25 2018, 9:05 PM

@bruns, do you know if Solid (and by extension, the Places panel) shows Windows disks when Dolphin is run on Windows?

Side note: does anyone run Dolphin on Windows? Do we distribute it at all?

As @davidedmundson said/hinted, Solid uses https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getlogicaldrives to show the drives in the Devices section (same as done using e.g. UDisks2 on Linux), but this is independent of listing *pathes* (Root, Home) in the Places section.

Thanks @bruns! So it looks like the Windows code can go after all. @davidedmundson?

Imo this patch can go in as it is, but before that I would like to see a patch to have the disk with the root file system getting specially marked (icon, color or text) for the case that several block devices are mounted at the same time.

ngraham added a subscriber: ndavis.Oct 1 2018, 8:57 PM

Imo this patch can go in as it is, but before that I would like to see a patch to have the disk with the root file system getting specially marked (icon, color or text) for the case that several block devices are mounted at the same time.

Yes, that's one of the action items for T8349: Improve Places panel usability and presentation. Before we can do it, we first needed a better disk icon, which we now have thanks to @ndavis (see D15853: Change drive-harddisk to more adaptable style)! If we want to signify root disk status via the icon, we still need to figure out what to do about emblems, which currently obscure most of the icon for the default 16x16 size. I submitted a patch for that, but it's run into controversy: D15866: Reduce emblem size for very small icons to prevent obscuring too much of the icon.

Baby steps...

davidedmundson resigned from this revision.Oct 1 2018, 8:59 PM
tcanabrava accepted this revision.Nov 7 2018, 3:29 PM
tcanabrava added a subscriber: tcanabrava.

too many +1 to not be accepted.

This revision is now accepted and ready to land.Nov 7 2018, 3:29 PM
ngraham planned changes to this revision.Nov 8 2018, 5:07 AM

This blows up the places tests; will fix them in this patch.

dcahal added a subscriber: dcahal.EditedNov 8 2018, 5:56 AM

I strongly advise against this. This change goes against Plasma's brand identity and will provoke a strongly negative reaction from our core audiences.

Plasma is strongly associated with powerful tools and high utility. Users also love having many options. Every time GNOME is criticized for "removing features," or not listening to feedback, we win big. Plasma is frequently praised for being the desktop environment that makes an effort to listen to its users.

I asked for some preliminary feedback on a large Discord server for Linux users. I made sure to specify that the change only applied to the Places link, and Root would remain visible in Devices.

Within two minutes of asking:

  • 4 users came out strongly against the change
  • 1 person was indifferent because they mostly used the terminal for most things outside of Home.
  • 2 people suggested removing the Desktop link from Places instead of Root.
  • 1 user asked for the removal of the Devices link to Root
  • 1 person had never noticed Root was listed under Devices

Only the terminal user believed the Places link presented a safety risk to GUI users. Three people said there was no safety risk because Dolphin does not allow the editing of system files.

Two users mentioned proprietary operating systems which provide a direct shortcut to the root directory.
One user said this change would make him "mad," and one person asked if the team thought users were stupid. One user said it sounded "like something GNOME would do, not KDE."

This was a quick 5 minute survey, but the feedback was very strong and even emotional.

My personal opinion aligns with many of these comments, and I too find this change out of character for Plasma.
I contribute to Promo and I am certainly not the most technical person here. I use this Places link many times a day. I consider the Devices link to be conceptually different, and it's much less convenient because my root partition is in the middle of my Devices list. I don't ever feel in danger of a misclick because Dolphin prevents any editing or deletion of system files.

I do not understand why this change provokes any negative reactions. Right now, the Places panel has two items that both lead to the exact same location. This patch simply removes one of them by default, and adding it again if you want is a 10 second operation. No functionality is lost; / can still be accessed with a single click on an item in the Places panel by default. There will still be a "direct shortcut to the root directory".

dcahal added a comment.Nov 8 2018, 6:07 AM

Are you referring to the mounted device under the Devices section? I'm unaware of how to get to the root directory in a single click using Places, if the existing link is removed.

My question to users addressed the Devices point and I also provided the user feedback re: Devices. As you can see, opinion on the Devices link was mixed, while they were strongly in favor of the Places link. The person who said they wanted Root removed from Devices explained that the first time he used Plasma, he saw it mounted there and was afraid he had installed his system onto the wrong partition.

In D15739#356008, @davidc wrote:

Are you referring to the mounted device under the Devices section?

Yes.

I'm unaware of how to get to the root directory in a single click using Places, if the existing link is removed.

Perhaps a visual aid will help. Here is how the Places panel looks today, as of KIO's git master:

This patch removed the red-circled item. The blue-circled item is still visible and still takes you to / when clicked on. Does that help? We are also thinking about improving the label given to the Root disk when it doesn't have a disk label. So instead of saying, "XXXX GiB Hard Drive", we could potentially make it say "Root drive" or "Root" or "XXXX GiB Hard Drive (Root)" or anything else, really.

dcahal added a comment.EditedNov 8 2018, 6:33 AM

Okay, I was only referring to the Places section, not the entire panel.

I do think changing the default label of Root's mounted device would prevent the confusion of the one user who wanted it removed from Devices. But that was only one of many criticisms of this change.

I think the most emotional respondents consider this change to be condescending. Painting / as "too dangerous" for the typical user, even though Dolphin already has restrictions that make system damage through the Dolphin GUI impossible. Removing the quick and easy Places link and relying on users to locate Root among their mounted physical devices gives the impression that we don't / to be easily accessible through Dolphin at all, even if people can't do anything once they get there.

I'm also worried this could make things more confusing for truly novice users, who may not realize there is a directory structure beyond Home at all. This can create many problems when trying to understand what the system is doing, or when troubleshooting.

Codezela added a subscriber: Codezela.EditedNov 8 2018, 7:58 AM

maybe there is 2 alternatives

  1. remove the icon from devices section and keep it in places section with some icon change in the top one
  2. keep it in the places section but change the label to root or system and it will always be top of all other hd

with the lock emblem in it and remove the places one
and we need to make the icons little bigger in this side bar
what do you think
or i come oo late

In D15739#356010, @davidc wrote:

I do think changing the default label of Root's mounted device would prevent the confusion of the one user who wanted it removed from Devices.

Okay great, I will submit a patch to improve that too.

I think the most emotional respondents consider this change to be condescending. Painting / as "too dangerous" for the typical user

Who did that? I didn't. In fact, it's the UI itself that currently portrays Root as dangerous by displaying it with a red icon. Red == dangerous in our design language. If anything, this change makes browsing / less dangerous. because it's no longer represented by an icon with a red color.

Removing the quick and easy Places link and relying on users to locate Root among their mounted physical devices gives the impression that we don't / to be easily accessible through Dolphin at all, even if people can't do anything once they get there.

This argument doesn't make any sense to me. It's still there available via one click. I continue to fail to see how this change makes it not "easily accessible through Dolphin at all". You can literally get to / with a single click. Also, the Devices section only shows internal partitions so it's not like there will generally be 15 devices crowding the list. And the entire purpose of coming up with a new icon for the rood partition was to make it easy to pick out the root partition when there are multiple partitions in the list. That change was specifically requested as a precondition for doing this.

I'm also worried this could make things more confusing for truly novice users, who may not realize there is a directory structure beyond Home at all. This can create many problems when trying to understand what the system is doing, or when troubleshooting.

"Truly novice users" don't have any desire to browse their computer's filesystem. The only people who are ever interested in that are technical experts, either actual or aspiring. I don't think this change will deter them in any way.

ndavis added a comment.EditedNov 8 2018, 3:56 PM

This argument doesn't make any sense to me. It's still there available via one click. I continue to fail to see how this change makes it not "easily accessible through Dolphin at all". You can literally get to / with a single click. Also, the Devices section only shows internal partitions so it's not like there will generally be 15 devices crowding the list. And the entire purpose of coming up with a new icon for the rood partition was to make it easy to pick out the root partition when there are multiple partitions in the list. That change was specifically requested as a precondition for doing this.

I'm neutral on this change, but it can be argued that having Root next to all the other Places bookmarks makes it easier to reach. That seems rather weak though because it's easy to add back to the Places section and people who might not want it in Places are currently unable to remove it.

ndavis added a comment.Nov 8 2018, 4:06 PM

maybe there is 2 alternatives

  1. remove the icon from devices section and keep it in places section with some icon change in the top one

You mean hide the root partition from Devices and keep the Root Places bookmark? That's what I currently do. I hide my / and /home partitions and use Places to get to those directories.

  1. keep it in the places section but change the label to root or system and it will always be top of all other hd with the lock emblem in it and remove the places one and we need to make the icons little bigger in this side bar what do you think or i come oo late

This confuses me. You say to keep it in Places, but remove it from Places? Keep what above other hard drives?

BTW, the sidebar icons do have adjustable sizes. Perhaps it should be easier to find, but if you right click in an area without something selectable (e.g., a section label), you can go to "Icon Size" and change the size to 16, 22, 32 or 48 px.

ngraham edited the summary of this revision. (Show Details)Nov 8 2018, 5:29 PM
Codezela added a comment.EditedNov 8 2018, 6:32 PM

maybe there is 2 alternatives

  1. remove the icon from devices section and keep it in places section with some icon change in the top one

You mean hide the root partition from Devices and keep the Root Places bookmark? That's what I currently do. I hide my / and /home partitions and use Places to get to those directories.

  1. keep it in the places section but change the label to root or system and it will always be top of all other hd with the lock emblem in it and remove the places one and we need to make the icons little bigger in this side bar what do you think or i come oo late

This confuses me. You say to keep it in Places, but remove it from Places? Keep what above other hard drives?

BTW, the sidebar icons do have adjustable sizes. Perhaps it should be easier to find, but if you right click in an area without something selectable (e.g., a section label), you can go to "Icon Size" and change the size to 16, 22, 32 or 48 px.

i know that the icons size is adjustable i mean make the default more bigger first time u run dolphin this icons is so small "out of the box experience"
the other thing i mean we may keep the root partion icon on the devices sections and make it fixed position above all other mounted drives so even any hd mounted the root icon still top i think the fixed postion make it better recognizable when needed
do u see this is my places section


can u tell me where is my root
and the phone icon is on top
i hope i explain well
sorry i know my English is not so good

ndavis added a comment.EditedNov 8 2018, 6:35 PM

i know that the icons size is adjustable i mean make the default more bigger first time u run dolphin this icons is so small "out of the box experience"
the other thing i mean we may keep the root partion icon on the devices sections and make it fixed position above all other mounted drives so even any hd mounted the root icons still top
i hope i explain well
sorry i know my English is not so good

It's ok, I understand now. Those seem like good ideas.

i know that the icons size is adjustable i mean make the default more bigger first time u run dolphin this icons is so small "out of the box experience"
the other thing i mean we may keep the root partion icon on the devices sections and make it fixed position above all other mounted drives so even any hd mounted the root icons still top
i hope i explain well
sorry i know my English is not so good

It's ok, I understand now. Those seem like good ideas.

sorry but i added some picture after

do u see this is my places section


can u tell me where is my root
and the phone icon is on top

+1 it's an entry I always hide when I setup someone's computer

you can still get to / on your machine in one click using the appropriate disk entry on the bottom of the Places panel.

Which I think shouldn't be there, but I'm too afraid of touching Solid's ignore rules ever again.

You advocate for

i know that the icons size is adjustable i mean make the default more bigger first time u run dolphin this icons is so small "out of the box experience"
the other thing i mean we may keep the root partion icon on the devices sections and make it fixed position above all other mounted drives so even any hd mounted the root icons still top
i hope i explain well
sorry i know my English is not so good

It's ok, I understand now. Those seem like good ideas.

sorry but i added some picture after

do u see this is my places section


can u tell me where is my root
and the phone icon is on top

It will have a new icon as Nate linked to earlier.

The image in this patch is out of date since it was added before other changes were made; I need to update it and will do so later today.

This blows up the places tests; will fix them in this patch.

It will also break the test in dolphin :(

This blows up the places tests; will fix them in this patch.

It will also break the test in dolphin :(

I know, and I'll probably have to fix those with ifdefs :(

meven added a subscriber: meven.Jun 4 2019, 6:12 PM

gentle ping @ngraham
How ironic ;)

Unless you would rather let someone else take care of this.

If you'd like to, I'm more than happy to hand it off. Removing Root is trivial, but adjusting the autotest has been a big pain. It explicitly looks for Root and I've had a tough time changing it appropriately. I've sunk a number of hours into it without success, so if you'd like to take over, please feel free.

meven updated this revision to Diff 59749.Jun 13 2019, 7:14 PM
  • Adapt test to remove Root in the Places by default
This revision is now accepted and ready to land.Jun 13 2019, 7:14 PM
meven commandeered this revision.Jun 13 2019, 7:15 PM
meven added a reviewer: ngraham.

Taking over this.
Dolphin test fix patch is coming too.

meven updated this revision to Diff 59750.Jun 13 2019, 7:24 PM
  • Adapt test to remove Root in the Places by default

<3

Nice huge diff! You see why I had trouble with it, maybe. :) I am very impressed that you pulled this off so quickly though!

meven updated this revision to Diff 59751.Jun 13 2019, 7:26 PM
  • Adapt test to remove Root in the Places by default
ngraham accepted this revision.Jun 13 2019, 9:38 PM

Lovely. :)

elvisangelaccio accepted this revision.Jun 16 2019, 8:24 PM
This revision was automatically updated to reflect the committed changes.
fvogt added a subscriber: fvogt.Jun 24 2019, 12:36 PM

I just noticed this in openQA runs from a live medium. There is no single-click way to get to the root directory anymore, as / is mounted as an overlayfs of a tmpfs and a squashfs container.

Not sure what the best way to improve this is, any idea?

I just noticed this in openQA runs from a live medium. There is no single-click way to get to the root directory anymore, as / is mounted as an overlayfs of a tmpfs and a squashfs container.

Not sure what the best way to improve this is, any idea?

So there's no entry in the Devices section that goes to /?

fvogt added a comment.Jun 24 2019, 1:01 PM

I just noticed this in openQA runs from a live medium. There is no single-click way to get to the root directory anymore, as / is mounted as an overlayfs of a tmpfs and a squashfs container.

Not sure what the best way to improve this is, any idea?

So there's no entry in the Devices section that goes to /?

No, there's in fact no devices section at all as it would be empty. The live cd itself is marked as ignored as there's nothing useful on there and it can't be ejected and the other layers are mounted from loop devices which udisks apparently ignores as well.

No, there's in fact no devices section at all as it would be empty. The live cd itself is marked as ignored as there's nothing useful on there and it can't be ejected and the other layers are mounted from loop devices which udisks apparently ignores as well.

That seems like a bug in the way the devices list is populated or displayed. There can't be no devices, that's silly.

fvogt added a comment.Jun 24 2019, 1:14 PM

No, there's in fact no devices section at all as it would be empty. The live cd itself is marked as ignored as there's nothing useful on there and it can't be ejected and the other layers are mounted from loop devices which udisks apparently ignores as well.

That seems like a bug in the way the devices list is populated or displayed. There can't be no devices, that's silly.

Well, the only mount of interest is the overlay mount of /, which is not backed by any (block) device but rather by two filesystems. AFAICT udisks doesn't care about this kind of mount, not sure what solid does.

Showing an overlay mount as "Device" is also somewhat wrong IMO.

Well, the only mount of interest is the overlay mount of /, which is not backed by any (block) device but rather by two filesystems. AFAICT udisks doesn't care about this kind of mount, not sure what solid does.

Showing an overlay mount as "Device" is also somewhat wrong IMO.

I'm saying that regardless of the technical details of the backend, it never makes sense to not show any devices. From the user's perspective, there is always a device of some sort, regardless of its underlying configuration. There can't not be a device. That doesn't make sense; software requires hardware.

fvogt added a comment.Jun 24 2019, 1:30 PM

Well, the only mount of interest is the overlay mount of /, which is not backed by any (block) device but rather by two filesystems. AFAICT udisks doesn't care about this kind of mount, not sure what solid does.

Showing an overlay mount as "Device" is also somewhat wrong IMO.

I'm saying that regardless of the technical details of the backend, it never makes sense to not show any devices. From the user's perspective, there is always a device of some sort, regardless of its underlying configuration. There can't not be a device. That doesn't make sense; software requires hardware.

Even if all (block) devices and their mountpoints were shown in the devices view, there would be no equivalent of "/". One loop device provides the read-only base for /, but that's actually more confusing than the current state as it looks like /, but is only a read-only view of "the past".

I guess solid needs to gain support for mountpoints not backed by devices?

Could be. I'm not at all an expert on Solid; I just think that it never makes sense to have a Places Panel with no Devices section. How to fix that is definitely best left up to people far smarter than me. :)

No, there's in fact no devices section at all as it would be empty. The live cd itself is marked as ignored as there's nothing useful on there and it can't be ejected and the other layers are mounted from loop devices which udisks apparently ignores as well.

That seems like a bug in the way the devices list is populated or displayed. There can't be no devices, that's silly.

Well, the only mount of interest is the overlay mount of /, which is not backed by any (block) device but rather by two filesystems. AFAICT udisks doesn't care about this kind of mount, not sure what solid does.

Showing an overlay mount as "Device" is also somewhat wrong IMO.

On Windows, we show "subst" drives as a Device, which is a bit similar to that, if solid could list those "virtual" devices so they're available on the places panel it would be useful for easy access to those mountpoints imho.

bruns added a comment.Jun 24 2019, 2:34 PM

Even if all (block) devices and their mountpoints were shown in the devices view, there would be no equivalent of "/". One loop device provides the read-only base for /, but that's actually more confusing than the current state as it looks like /, but is only a read-only view of "the past".

I guess solid needs to gain support for mountpoints not backed by devices?

It actually already does, via the fstab backend (currently network (SMB/NFS) and various fuse mounts). Fstab in this case also includes everything in MTAB, should be quite easy to extend this.

What is the output of cat /proc/self/mounts (fell free to remove anything irrelevant, like cgroups)?

fvogt added a comment.Jun 24 2019, 2:56 PM

Even if all (block) devices and their mountpoints were shown in the devices view, there would be no equivalent of "/". One loop device provides the read-only base for /, but that's actually more confusing than the current state as it looks like /, but is only a read-only view of "the past".

I guess solid needs to gain support for mountpoints not backed by devices?

It actually already does, via the fstab backend (currently network (SMB/NFS) and various fuse mounts). Fstab in this case also includes everything in MTAB, should be quite easy to extend this.

What is the output of cat /proc/self/mounts (fell free to remove anything irrelevant, like cgroups)?

The entries involved in / are these:

/dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
/dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
/dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
LiveOS_rootfs / overlay rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work 0 0
bruns added a comment.Jun 24 2019, 3:24 PM

It actually already does, via the fstab backend (currently network (SMB/NFS) and various fuse mounts). Fstab in this case also includes everything in MTAB, should be quite easy to extend this.

What is the output of cat /proc/self/mounts (fell free to remove anything irrelevant, like cgroups)?

The entries involved in / are these:

/dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
/dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
/dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
LiveOS_rootfs / overlay rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work 0 0

Hm, loop1 is the lowerdir of the overlay - how are loop0 and sr0 involved, are these the backing files?

Though, the relevant part is mntent.mnt.type == "overlay" and mntent.mnt_dir == "/". Matching for "overlay" is probably sufficient.

fvogt added a comment.EditedJun 24 2019, 3:29 PM

It actually already does, via the fstab backend (currently network (SMB/NFS) and various fuse mounts). Fstab in this case also includes everything in MTAB, should be quite easy to extend this.

What is the output of cat /proc/self/mounts (fell free to remove anything irrelevant, like cgroups)?

The entries involved in / are these:

/dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
/dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
/dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
LiveOS_rootfs / overlay rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work 0 0

Hm, loop1 is the lowerdir of the overlay - how are loop0 and sr0 involved, are these the backing files?

/dev/sr0 contains a squashfs, which is visible as /dev/loop0.
/dev/loop0 contains an ext4 image, which is visible as /dev/loop1.

So it's doubly indirect.

Though, the relevant part is mntent.mnt.type == "overlay" and mntent.mnt_dir == "/". Matching for "overlay" is probably sufficient.

If adding such a special case makes sense, yes. I'd even argue about 'mntent.mnt_dir == "/" && !isKnownFilesystem(mntent)' or something like that to ensure that an entry for / is always provided.

bruns added a comment.Jun 24 2019, 4:51 PM

If adding such a special case makes sense, yes. I'd even argue about 'mntent.mnt_dir == "/" && !isKnownFilesystem(mntent)' or something like that to ensure that an entry for / is always provided.

Guaranteeing there will be an "/" entry is hard, as this can be provided by different backends. And I am not sure how e.g. dolphin would deal with multiple devices providing "/".