[Kde-pim] Review Request: Fix 'read-only' being checked in single file resource config dialog when user didn't select it

[Kevin Krammer]

> On Dec. 3, 2011, 10:30 a.m., Kevin Krammer wrote:
> > Looks good to me.
> > 
> > The only problem I see (but it is of course still better than the current code) is that this additional information (read-only by user choice) is discarded with the config UI.
> > Maybe we should not save the flag in "ON" state if it is a result of file permissions.
> 
> David Jarvie wrote:
>     The potential problem with not storing the read-only state if it a result of file permissions, is that the user may have wanted it to be read-only even if later the file's permissions change to read-write. If the dialog shows it as read-only, I think it is safer to save it as read-only.
> 
> Kevin Krammer wrote:
>     The user could still set it to read-only right?
>     I mean if we decouple the read-only thing from the file access rights. The resource would take setting and file permissions into account when setting collection rights.
>     read-only ON -> read-only collection. read-only OFF -> collection rights == file access rights
> 
> David Jarvie wrote:
>     Perhaps I wasn't clear. If the user chooses a read-only file and sees that the read-only checkbox is (automatically) checked, he/she might think, "Good, it's marked as read-only - that's what I want it to be." If, later, the file permissions change, the user might not realise that this might affect the collection's rights, and would still think, "I set up that collection as read-only. I'm happy." But using your suggestion, the collection permissions would now have changed to read-write - not what this user expected.
>     
>     So I think it's safer just to mark it as read-only, whether or not the user actually checked the read-only box.
>     
>     The only unambiguous way to approach this would be to have a separate indication in the dialog about the actual read-only status of the resource, in addition to the desired (user-checkable) state. But this might just make things more complicated than is really warranted.

Sorry, you were clear, but I think I wasn't.

You are absolutely right that due to the automatic checking of the read-only option a user would assume that the resource stayed that way.

I think this automated setting of a user choosable option is wrong, it makes the resource lose the vital information about whether the user wanted it to be read-only or not.
What I am not sure about is whether we really need an indication that the file is currently not writable, especially since it is only a fact at the moment of the check and could change at any time.

So IMHO we should just remove this auto-checking.


- Kevin


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


On Dec. 2, 2011, 7:47 p.m., David Jarvie wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103310/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2011, 7:47 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Currently, when typing the file name in the single file resource config dialog, if the entered text happens to match a read-only file which already exists, the read-only box is checked and disabled. When the user continues to type, or deletes some of the entered text, so that the name no longer matches a read-only file, the read-only box is re-enabled but is not unchecked.
> 
> This patch unchecks the read-only box when the file name no longer matches an existing read-only file, provided that the user didn't previously check it.
> 
> This prevents unintentional setting of 'read-only', and removes user confusion as to why read-only has become set when the user didn't select it.
> 
> 
> Diffs
> -----
> 
>   resources/shared/singlefileresourceconfigdialogbase.h 6f3aa86 
>   resources/shared/singlefileresourceconfigdialogbase.cpp ab73499 
> 
> Diff: http://git.reviewboard.kde.org/r/103310/diff/diff
> 
> 
> Testing
> -------
> 
> Tested config dialog when creating new ICal and KAlarm calendar file resources.
> 
> 
> Thanks,
> 
> David Jarvie
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/