Review Request 111120: Also change the view mode, when the view properties settings file is not writeable.

[Emmanuel Pescosta]

> On June 19, 2013, 2:50 p.m., Frank Reininghaus wrote:
> > Thanks Emmanuel for looking into this issue!
> > 
> > The class ViewProperties does have support for storing the view properties in the user's home directory if the .directory file is not writeable. And this works fine for me if I remove the write permissions from any .directory file.
> > 
> > Could you please describe how you managed to reproduce the "cannot change view mode" bug? It seems that the lack of write permissions is not sufficient (or maybe, the code that detects if we have write permissions is broken?). Thanks.
> 
> Emmanuel Pescosta wrote:
>     > The class ViewProperties does have support for storing the view properties in the user's home directory if the .directory file is not writeable.
>           if (!dirInfo.isWritable() || (fileInfo.exists() && !(fileInfo.isReadable() && fileInfo.isWritable())) || !isPartOfHome(m_filePath)) {
>                 m_filePath = destinationDir("local") + m_filePath;
>           }
>     
>     Dolphin always saves the .directory files in the user's home directory for non home paths. So I think there is something wrong with the write 
>     permission of the .directory file in the user's home directory (maybe the usage of sudo?!).
>     
>     > Could you please describe how you managed to reproduce the "cannot change view mode" bug?
>     By making the .directory file read-only in the user's home directory.
>     
>     Btw.: This patch fixes the same bug for ro mounted home partitions. Maybe useful for some special environments?
> 
> Frank Reininghaus wrote:
>     Thanks for the explanation! Before we do anything about the issue, I'd like to know if the "no write permissions for (something inside) the home directory" thing is really the cause of the bug.
>     
>     If it is, your patch might be a good way to fix it, but I'm still not sure if we should really do it. I think that many other parts of KDE won't really work as expected when the home directory is not writeable, so I'm not sure if we should harm the readability of our code (it's not really clear what the 'bool' parameter does if you just see a setMode(XYZ, true) somewhere in the code) or just tell the user to give himself write permissions.
>     
>     BTW, I'm not sure if 'sudo' can explain how this happened. When running 'sudo dolphin', shouldn't it actually store all ViewProperties in root's home directory?
> 
> Emmanuel Pescosta wrote:
>     > Before we do anything about the issue, I'd like to know if the "no write permissions for (something inside) the home directory" thing is really the cause of the bug.
>     Ok.
>     
>     > When running 'sudo dolphin', shouldn't it actually store all ViewProperties in root's home directory?
>     Yes in the default case, but when you set $HOME to the normal user's home directory .... I don't know what the exact problem was, but something strange must happened with the permissions. (Sudo was just a guess ;)
> 
> Frank Reininghaus wrote:
>     I thought again about this issue, and I think that we should fix it - even if having non-writeable .directory files inside ~ is a bit weird, I think that clicking a "view mode" button in the tool bar should always change the view mode.
>     
>     I think that a slightly more elegant solution is to bypass the "Save the view properties to disk, and then re-read them from there" idea: http://paste.kde.org/779582/
>     
>     This also removes the need for saving the ViewProperties explicitly - they are stored on destruction anyway. What do you think?

> What do you think?
Yes this code is better. "Ship it" from my side ;)


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111120/#review34680
-----------------------------------------------------------


On June 19, 2013, 2:33 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111120/
> -----------------------------------------------------------
> 
> (Updated June 19, 2013, 2:33 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The problem is, that the new view mode will be saved to the view properties file on view mode change. 
> After saving the new view mode, Dolphin re-reads the view properties file to get the new view
> mode from this file to apply it to the view. 
> 
> Change View Mode => Save View Mode to the file => Read View Mode from the file => Apply it to the View
> 
> When the view properties file is not writeable, the view mode can not be changed with the current 
> implementation.
> 
> This patch fixes this problem, because the view mode can always be changed, also if the properties file
> is not writable. (Dolphin tries to save the View Mode on view mode change)
> 
> Change View Mode => Apply it to the View + Save View Mode to the file if possible
> 
> 
> This addresses bug 318534.
>     http://bugs.kde.org/show_bug.cgi?id=318534
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/dolphinview.h 62b5df7 
>   dolphin/src/views/dolphinview.cpp d69d664 
> 
> Diff: http://git.reviewboard.kde.org/r/111120/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130621/e26958ce/attachment.htm>