Bug 136003 - PROTECT SHEET: Can't delete rows, despite of allow to deletes
Summary: PROTECT SHEET: Can't delete rows, despite of allow to deletes
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Sahil Gautam
URL:
Whiteboard: target:24.8.0
Keywords:
Depends on:
Blocks: Cell-Sheet-Protection
  Show dependency treegraph
 
Reported: 2020-08-22 00:35 UTC by sawakaze
Modified: 2023-12-22 07:17 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
ScreenCast (888.25 KB, video/x-matroska)
2020-08-22 00:36 UTC, sawakaze
Details

Note You need to log in before you can comment on or make changes to this bug.
Description sawakaze 2020-08-22 00:35:31 UTC
Description:
following steps, I can't delete row on sheet.
detail is screencast.

Steps to Reproduce:
1. open calc
2. Tools - Protect Sheet
-> launch Protect Sheet dialog
3. check "Delete rows" to ON 
4. click OK
5. open Menu - Sheet - Delete Rows

Actual Results:
Menu - Sheet - Delete Rows is gray out
user can't delete rows on sheet

Expected Results:
Menu - Sheet - Delete Rows is not gray out
user can delete rows on sheet


Reproducible: Always


User Profile Reset: No



Additional Info:
I confirm 

Version: 7.1.0.0.alpha0+
Build ID: <buildversion>
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3
Locale: ja-JP (ja_JP.UTF-8); UI: en-US
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2020-08-19_02:56:36
Calc: threaded
Comment 1 sawakaze 2020-08-22 00:36:14 UTC
Created attachment 164545 [details]
ScreenCast
Comment 3 Roman Kuznetsov 2020-08-26 06:35:40 UTC Comment hidden (obsolete)
Comment 4 himajin100000 2020-09-06 15:52:37 UTC
https://gerrit.libreoffice.org/c/core/+/102115
Comment 5 QA Administrators 2022-09-07 03:59:03 UTC Comment hidden (obsolete)
Comment 6 Roman Kuznetsov 2022-09-07 09:07:02 UTC
Still repro in

Version: 7.5.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 3e544b6938ee509a4f6df4c2e2996d71ce072506
CPU threads: 8; OS: Windows 10.0 Build 19043; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: en-US
Calc: CL threaded

There are code pointers in Comment 2 and even some patch in Comment 4 where someone can try start to fix it. So let's set it as EasyHack
Comment 7 Kira Tubo 2023-09-01 06:38:24 UTC
Still reproducible in:

Version: 24.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 695ae365dcab7c7dd59b39411299c5c200081885
CPU threads: 6; OS: Windows 10.0 Build 22621; UI render: Skia/Raster; VCL: win
Locale: en-US (en_US); UI: en-US
Calc: CL threaded
Comment 8 Sahil Gautam 2023-12-18 01:21:23 UTC
* The Real Bug
This bug "Delete Rows/Delete Columns disabled even when checked in the Protect Sheet dialog box" is not a bug in
itself, but is a consecuence of all cells being protected by default. Deleting rows/columns is not allowed,
even when it is checked in the Protect Sheet dialog box, if the cells are protected. This is what the code says, and
what the release notes say https://wiki.documentfoundation.org/ReleaseNotes/5.4#Calc, and what the Buovjaga said
in  https://bugs.documentfoundation.org/show_bug.cgi?id=43535. And tdf#43535 has been solved.

This issue can simply be solved by making all cells unprotected by default in calc sheet.


a) Unprotect all the cells on a sheet, and then go ahead to protect the sheet, not checking the delete row/column
option. What is expected is that the =Delete rows(columns)= options are disabled, but they are not.
(Why should they be disabled, because the sheet is protected, and we didn't check delete rows/columns.)

b) Create a new sheet, and unprotect all the cells by selecting all, then =Right-click > Format Cells > Cell Protection=,
uncheck =Protected=. then protect only one cell, and remember which one was it. Then protect the sheet allowing
delete rows/columns. Select the protected cell, and check the =Delete rows/columns= options under sheet tab.
They are disabled as expected, as we are on a row and a column, both of which have a protected cell (the
selected one). Now select the cell on the right of the protected cell, and check the options under sheet tab.

What is expected is for the delete rows option to be disabled, and delete columns to be enabled. But both are
enabled. Clicking on delete rows does say protected cells in the sheet, but it should be disabled. Delete columns
works fine.

Summary:
Unprotect everything, and then protect the sheet without allowing delete rows/col, the options are not disabled.
Have one cell in the protected row/col and don't have the focus on it, that respective option(s) is not disabled.


Why is cell protection enabled by default (for all the cells)?
Comment 9 ady 2023-12-18 02:00:33 UTC
(In reply to Sahil Gautam from comment #8)
> Why is cell protection enabled by default (for all the cells)?

The reasoning is to make it easier for basic/unaware users to be able to protect any kind of editions. When more advance conditions or cases are needed, users are able to learn about features and procedures.

Imagine this would be backward. A newbie would have to follow some complicated tutorial, multiple kinds of possible actions that can or cannot be allowed on cells, ranges, worksheets or entire workbooks, and so on. Moreover, such newbie user would have first to be aware of all those possibilities.

So, having the cells "ready" for the simpler actions allows an easier introduction to the basic feature, with a relatively simple procedure. For more advance combinations of what would be allowed (and what would not) and in which areas of the spreadsheet, the procedure requires more knowledge, experience and skills.

I am not saying that the current methods are perfect, but, generally speaking, the basic level of features should rather be "easier" for newbies than the other way around. That is, without making it unnecessarily hard for experienced users.

Perhaps the solution is more related to (better) documentation, easier tutorials and improved UX methods/procedures, rather than changing the default settings (which are like they are since... "forever", not just in LO Calc but probably in many/most/every spreadsheet tool).
Comment 10 Sahil Gautam 2023-12-18 02:09:15 UTC
(In reply to ady from comment #9)

> Perhaps the solution is more related to (better) documentation, easier
> tutorials and improved UX methods/procedures, rather than changing the
> default settings (which are like they are since... "forever", not just in LO
> Calc but probably in many/most/every spreadsheet tool).

But I think the inconsistent behaviour needs to be addressed (where only one cell of the row is protected, but the delete rows option still shows as enabled). In some other bug report then, as this one is not a bug anymore, considering the defaults?
Comment 11 Buovjaga 2023-12-18 08:31:41 UTC
(In reply to Sahil Gautam from comment #8)
> * The Real Bug
> This bug "Delete Rows/Delete Columns disabled even when checked in the
> Protect Sheet dialog box" is not a bug in
> itself, but is a consecuence of all cells being protected by default.
> Deleting rows/columns is not allowed,
> even when it is checked in the Protect Sheet dialog box, if the cells are
> protected. This is what the code says, and
> what the release notes say
> https://wiki.documentfoundation.org/ReleaseNotes/5.4#Calc, and what the
> Buovjaga said
> in  https://bugs.documentfoundation.org/show_bug.cgi?id=43535. And tdf#43535
> has been solved.
> 
> This issue can simply be solved by making all cells unprotected by default
> in calc sheet.

Good catch! In bug 43535 comment 14 Kohei says about inserting "I took that behavior from how Excel handles it". I just tested with online Excel and the deletion behaves the same way with the same protection options.

So yeah, maybe UX team can think about how to convey this better to users.
Comment 12 Heiko Tietze 2023-12-19 09:08:37 UTC
We could separate those options that apply to unprotected cells only from the rest, and perhaps give a clue what to do. Or just always allow to delete rows/cols with protected cells.
Comment 13 Sahil Gautam 2023-12-19 09:28:59 UTC
There is no easy way to unprotect all the cells of a sheet. So the user has to go the long way, select all the cells, then right click > format cells > cell protection, and then uncheck. That too works only when the sheet is empty (doesn't have anything in any cell).

Test it like this
create an empty sheet, then check cell protection for all the cells by selecting all, then right click > format cells > cell protection (protected by default). uncheck it, and then check if protection is turned off for all the cells or not, by going to random cells and checking cell protection. (it works for empty sheet i.e. you would be able to unprotect all the cells.)

then again close this sheet and create a new sheet, write something in some cells (say 1 2 3 4 5 6 in the first 6 cells of column A). Then go ahead and try to unprotect all the cells again using the same method above. It doesn't work. 

So there is a need to have a button (or something similar) which can toggle cell protection for either the whole sheet, or the selection. (I don't know if there is any).

And Excel doesn't allow deleting rows with protected cells, so I think would be better to keep the behaviour consistent with it. (I mean it might be a big deal for some people)
Comment 14 Buovjaga 2023-12-19 09:38:32 UTC
(In reply to Sahil Gautam from comment #13)
> There is no easy way to unprotect all the cells of a sheet. So the user has
> to go the long way, select all the cells, then right click > format cells >
> cell protection, and then uncheck. That too works only when the sheet is
> empty (doesn't have anything in any cell).

Styles to the rescue: Sidebar - Styles - Cell Styles (default view) - Right-click Default - Edit Style... - Cell Protection. Uncheck "Protected".
Comment 15 Heiko Tietze 2023-12-19 09:41:01 UTC
How about changing the labels to "Delete columns/rows (for unprotected cells)"?
Comment 16 Sahil Gautam 2023-12-19 10:09:45 UTC
Thought for a sold 10 minutes.(In reply to Heiko Tietze from comment #15)
> How about changing the labels to "Delete columns/rows (for unprotected
> cells)"?

Thought for a solid 10 minutes. Wouldn't it mean "Oh this feature allows deleting rows which have unprotected cells only". This doesn't talk about sheet protection. If the sheet is not protected, then delete is allowed even when rows have protected cells. This would be misleading for the "Unprotected Sheet" situation. (Or is it like cell protection doesn't mean anything if sheet is unprotected, in which case it's fine by my side).

Maybe a better way to make the user aware of the fact that all the cells are protected by default, and even if you are allowing delete row/col, you might not able to do so for the stated reason. Something like a small note in the sheet protection dialog box before the after the checkboxes, and before the ok button.
Comment 17 ady 2023-12-19 16:22:58 UTC
(In reply to Sahil Gautam from comment #13)
> then again close this sheet and create a new sheet, write something in some
> cells (say 1 2 3 4 5 6 in the first 6 cells of column A). Then go ahead and
> try to unprotect all the cells again using the same method above. It doesn't
> work. 

There are several reports already about similar failures. ATM, changing some attribute to "all cells to the right of active area, or lower to it" is failing, in the sense that you _could_ try to change/edit something, but it does not get actually changed.

IIRC, these types of problems started when some kind of "performance/shortcut method" was applied, after/because of the lag introduced by 16k columns in LO 7.4; i.e. avoid applying some property/attribute to "everything" and instead apply it to the active area only. Nice thought, with a lot of problems.
Comment 18 ady 2023-12-19 18:05:03 UTC
(In reply to Sahil Gautam from comment #16)
> Or is it like cell protection doesn't mean
> anything if sheet is unprotected, in which case it's fine by my side).

AFAIK, that has always been the case, not just in Calc. Cell's protection is relevant for a protected worksheet. Until the relevant worksheet is set as protected, cell's protection should not have any effect.
Comment 19 Commit Notification 2023-12-22 07:15:27 UTC
Sahil committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/68d074fb910de7298cbefb6a3c3e192dae201837

tdf#136003 Make Delete Rows/Columns labels more descriptive

It will be available in 24.8.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 Heiko Tietze 2023-12-22 07:17:20 UTC
(In reply to Commit Notification from comment #19)
> Sahil committed a patch related to this issue.

...in which he amended the UNO command labels with "(for unprotected cells)". While this is not the ultimate solution it might at least give a clue and make user read the help.