Bug 143759 - Inaccurate results when searching for empty cells with regex ^$
Summary: Inaccurate results when searching for empty cells with regex ^$
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
5.2.0.4 release
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:7.3.0 target:7.1.6 target:7.2.1
Keywords:
Depends on:
Blocks: Find-Search
  Show dependency treegraph
 
Reported: 2021-08-06 13:30 UTC by Stéphane Guillou (stragu)
Modified: 2021-08-17 14:02 UTC (History)
0 users

See Also:
Crash report or crash signature:


Attachments
sample ODS for following steps to reproduce (7.63 KB, application/vnd.oasis.opendocument.spreadsheet)
2021-08-06 13:32 UTC, Stéphane Guillou (stragu)
Details
resulting selection after following steps to reproduce in LO 7.3 alpha0+ (11.29 KB, image/png)
2021-08-06 13:33 UTC, Stéphane Guillou (stragu)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stéphane Guillou (stragu) 2021-08-06 13:30:23 UTC
Description:
Searching for empty cells with a regular expression (inside a selection) results in inaccurate results:
- empty cells that are part of the selection are not matched
- empty cells that are not part of the selection are matched

Steps to Reproduce:
1. Open attachment
2. Select range that goes beyond all cells with data, on all sides. For this example, use the range A1:F6
3. Open Search & Replace dialog
4. Tick "Regular Expression" and "Current selection only"
5. Use the regular expression "^$" in the "Find" field (without quotes)
6. Click "Find All"

Actual Results:
In the resulting selection:
- empty cells that are part of the selection are not matched (e.g. in row 1 and column A)
- empty cells that are not part of the selection are matched (e.g. in column D)

Expected Results:
All cells that are empty in the selected range are matched. Nothing more, nothing less.


Reproducible: Always


User Profile Reset: No



Additional Info:
Observed in:

Version: 6.2.5.2
Build ID: 1ec314fa52f458adc18c4f025c545a4e8b22c159
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3; 
Locale: en-AU (en_AU.UTF-8); UI-Language: en-US
Calc: threaded

and

Version: 7.0.6.2
Build ID: 00(Build:2)
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Ubuntu package version: 1:7.0.6-0ubuntu0.20.04.1_lo1
Calc: threaded

and

Version: 7.2.0.2 / LibreOffice Community
Build ID: 614be4f5c67816389257027dc5e56c801a547089
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded

and

Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: 1dd4a80fa076bedb3a82821517036bad8dd79857
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2021-07-26_22:41:19
Calc: threaded
Comment 1 Stéphane Guillou (stragu) 2021-08-06 13:32:32 UTC
Created attachment 174110 [details]
sample ODS for following steps to reproduce
Comment 2 Stéphane Guillou (stragu) 2021-08-06 13:33:28 UTC
Created attachment 174111 [details]
resulting selection after following steps to reproduce in LO 7.3 alpha0+
Comment 3 Eike Rathke 2021-08-06 15:08:11 UTC
Only the marked range from D6 down is an error.
Empty cells outside of the actual rectangular data area (i.e. no occupied cell at all in an empty column) are *never* searched (because you certainly don't want to search a billion empty cells for an unnecessarily large selection). In the sample file the actual data area is B2:E5. You can observe the intended behaviour when hitting Find Next repeatedly.
Comment 4 Stéphane Guillou (stragu) 2021-08-07 00:00:54 UTC
Thanks Eike.
About the range outside of the data area: that makes sense that it is not currently searched currently.
It is even commented about in the code: https://cgit.freedesktop.org/libreoffice/core/commit/?id=69ac605191860aceee09f1147a5234222d1b3300
But wondering if ScTable::SearchAndReplaceEmptyCells could be improved by adding an extra step:
1. remove the range outside the data area before searching (current behaviour);
2. search inside the data area (current behaviour);
3. add the outside selection back into returned selection (added step).
That wouldn't add unnecessary overhead, but would result in a more expected behaviour, in my opinion.

In any case, the most important issue is obviously the second part about returning selected cells that weren't originally part of the selection.

Reproduced in 5.2.0 as well, so assuming the behaviour started with the commit linked above.

Version: 5.2.0.0.alpha1
Build ID: 902b28a39528b6c92602e9b521a1d0861be1caf9
CPU Threads: 4; OS Version: Linux 5.4; UI Render: default; 
Locale: en-AU (en_AU.UTF-8)
Comment 5 Eike Rathke 2021-08-08 15:14:04 UTC
I'd refrain from changing the logic there. The same is used for displaying the result list and replacing empty cells (which btw currently wrongly replaces the entire column in this scenario). If the initial empty ranges selection was added that wouldn't match the replacement results anymore. It's questionable whether replacing such extraneous selection would be actually wanted. In any case, I wouldn't change that to fix this bug.
Comment 6 Commit Notification 2021-08-08 17:38:46 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/15960931988cfd898e14a12d6b9cddaf6d8b0ade

Resolves: tdf#143759 Limit empty search's empty column to actual search range

It will be available in 7.3.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 7 Eike Rathke 2021-08-08 18:39:07 UTC
Pending review
https://gerrit.libreoffice.org/c/core/+/120134 for 7-2
https://gerrit.libreoffice.org/c/core/+/120135 for 7-1
Comment 8 Commit Notification 2021-08-08 20:50:57 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

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

Resolves: tdf#143759 Limit empty search's empty column to actual search range

It will be available in 7.1.6.

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 9 Commit Notification 2021-08-08 20:51:10 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

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

Resolves: tdf#143759 Limit empty search's empty column to actual search range

It will be available in 7.2.1.

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 10 Commit Notification 2021-08-09 00:25:14 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1c5b3cb3dd4dab6b0db409b6cc75b3111103820f

Related: tdf#143759 Display results of find empty or replace to empty

It will be available in 7.3.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 11 Commit Notification 2021-08-10 09:25:03 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/95d619f92b32a576b0e0417508b66147b7f05afd

tdf#143759: sc: Add UItest

It will be available in 7.3.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 12 Stéphane Guillou (stragu) 2021-08-10 13:20:02 UTC
Fabulous, thank you Eike and Xisco!

Verified as fixed in:

Version: 7.3.0.0.alpha0+ / LibreOffice Community
Build ID: b2130ad3fda841c68a0436fbddf29bcedede0af5
CPU threads: 8; OS: Linux 4.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2021-08-09_13:03:07
Calc: threaded
Comment 13 Commit Notification 2021-08-17 14:02:17 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/14f259faf4b5620f520ca358edb3b03f2368db9d

Related: tdf#143759 Display results of find empty or replace to empty

It will be available in 7.2.1.

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.