Created attachment 164545 [details]
ScreenCast

https://opengrok.libreoffice.org/xref/core/officecfg/registry/data/org/openoffice/Office/UI/CalcCommands.xcu?r=7b480207#1054

https://opengrok.libreoffice.org/xref/core/sc/sdi/cellsh.sdi?r=02cac3ee#123

https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh.cxx?r=e67657d5#123

there is no SID_DEL_ROWS in switch statement.

therefore bEditable is important.

https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh.cxx?r=e67657d5#313

https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh.cxx?r=e67657d5#316

https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh.cxx?r=e67657d5#106

https://opengrok.libreoffice.org/xref/core/sc/source/core/data/table2.cxx?r=75f398b2#2428

while the menu items like FID_INS_ROWS_BEFORE relies on IsEditActionAllowed

https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/cellsh.cxx?r=e67657d5#241

https://gerrit.libreoffice.org/c/core/+/102115

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

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

* 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)?

(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).

(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?

(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.

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.

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)

(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".

How about changing the labels to "Delete columns/rows (for unprotected cells)"?

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.

(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.

(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.

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.

(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.

(In reply to Heiko Tietze from comment #20)
> ...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.

The widening of the menu is IMNSHO too much to be justified by this situation. We can bring up an error dialog, or put something on the status bar, or whatever. Please reopen this bug.

(In reply to Eyal Rozenberg from comment #21)
> The widening of the menu is IMNSHO too much to be justified by this
> situation. We can bring up an error dialog, or put something on the status
> bar, or whatever. Please reopen this bug.
Although the information added to the label is very useful in some cases, I think it is better to give relevant feedback depending on the situation.
When discussed in design, maybe the choice is that it deserves a new bug report?

Sahil Gautam committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d18fbc0c20180f6856e9e47eaa97e3763dd4d8e9

Revert "tdf#136003 Make Delete Rows/Columns labels more descriptive"

It will be available in 25.2.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.

We discussed the topic in the design meeting and decided to rather keep the command itself short, aka revert the patch, but make it more clear at the protect sheet dialog what allowing to delete row/col means. That could be "[ ] Delete columns with unprotected cells".

Code pointer: sc/uiconfig/scalc/ui/protectsheetdlg.ui

Sahil Gautam committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/df175a50b687df3abea37515f1154f40a7920319

tdf#136003 Change labels in the protect sheet dialog

It will be available in 25.2.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.

> Sahil Gautam committed a patch related to this issue.
> It has been pushed to "master":
> 
> https://git.libreoffice.org/core/commit/
> df175a50b687df3abea37515f1154f40a7920319

It looks like this commit is adding the extra text, not removing it - Sahil, can you double-check?

(In reply to Eyal Rozenberg from comment #26)
> > Sahil Gautam committed a patch related to this issue.
> > It has been pushed to "master":
> > 
> > https://git.libreoffice.org/core/commit/
> > df175a50b687df3abea37515f1154f40a7920319
> 
> It looks like this commit is adding the extra text, not removing it - Sahil,
> can you double-check?

Please don't be so hasty. You don't need to change the status every time you ask something. The commit changes strings in the dialog as it was agreed in the design call. You were looking for commit d18fbc0c20180f6856e9e47eaa97e3763dd4d8e9 instead.