dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> kpropertiesdialog.cpp:994
> }
> d->nameArea = lab;
> } else {
What about this code path? You broke it by removing the call to
`grid->addWidget(d->nameArea, curRow++, 2);` in that case.
> kpropertiesdialog.cpp:998
> KFileItemListProperties itemList(KFileItemList() << item);
> setFileNameReadOnly(!itemList.supportsMoving());
> + // if supports writing then KLineEdit, else QLabel
This method accesses d->m_lined which isn't created yet (you moved that below),
so it's null always, and this call would now be dead code.
The proper fix, I think, is to reuse the above code that creates a QLabel
instead of a KLineEdit (currently that's for other cases like selecting
multiple files, or selecting the root directory, etc.), and go into that
condition (the `if` on line 987) also when the directory doesn't support
renaming, or the files can't be moved.
No point in having two different code paths that both create the same QLabel,
differently, and with bugs in both, in this version of the patch...
In fact I think your patch should end up removing this call here, no?
If we catch all cases of "renaming forbidden" upfront, this isn't needed
anymore.
> kpropertiesdialog.cpp:1000
> + // if supports writing then KLineEdit, else QLabel
> + if (itemList.supportsWriting()) {
> + d->m_lined = new KLineEdit(d->m_frame);
That test is about writing to the files themselves. To test if the folder
forbids renaming, it's the folder that you should be testing for write access,
not the file(s).
[at least on Unix; I don't remember the exact semantics on Windows].
Testcase: create a dr-xr-xr-x (0555) folder, and create a -rw-r--r-- (0644)
file in it. Renaming is forbidden, if you can write to the file itself, so you
should get the label and not the lineedit..
Arguably a possible ("proper") fix would be for
KFileItemListProperties::supportsMoving to return false when the directory is
readonly, in fact. If that was the case, you would get a readonly lineedit
already.
(or do you? the patch description is unclear)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D14610
To: shubham, rkflx, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
