Bug 133336 - calc: fileopen: filter: irregular reading and saving of filter definitions in ods format affecting all filters
Summary: calc: fileopen: filter: irregular reading and saving of filter definitions in...
Status: RESOLVED DUPLICATE of bug 133529
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.5.2 release
Hardware: All All
: medium normal
Assignee: b.
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-24 07:07 UTC by b.
Modified: 2020-06-20 23:53 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
file 'first save' with correct filter definitions (8.74 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-05-24 07:13 UTC, b.
Details
file 'second save' with wrong filter definitions (9.07 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-05-24 07:14 UTC, b.
Details
Step 1 and 2 File (7.65 KB, application/vnd.oasis.opendocument.spreadsheet)
2020-05-25 19:44 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description b. 2020-05-24 07:07:12 UTC
Description:
filter conditions are saved incorrectly in ods format starting with the second save. 

e.g.: first save no tag 'table:orientation="column"', field-number(s) "0" and "1", 

after save - close - reopen - save without modifications: 

range has an additional tag 'table:orientation="column"', field-number(s) "-1" and "0", 

starting with the reload of the first save access to filtered fields by macros / basic fail. 

preconditions: the filtered range should not start at fields A1-B2-C3-D4-E5-F6 ..., in such cases you get the wrong flag 'orientation', but can't spot the 'field-number' errors because 'row-offset' and 'column-offset' have the same value, 

imho the second save is wrong and violates the tdf standard for documents as it applies a filtering condition to a field '-1' off relative to the addressed range and thus outside of it, 

imho the initial fail is the read/load of the first save, after that the '.field' values accessible by basic/macros are messed up, 

imho fails like this are critical as they undermine any downstream work, analysis and fixing, 

will apply sample documents in next comments, observe that on loading them you have a re-read condition, thus don't get the initial correct state, it's better to check them with a packer and open the content.xml file inside and / or create the files from scratch as described in 'steps', 

this bug affects 'autofilter' too, as well as advanced filters, 

this bug is related to bugs filed during my approach to the problem, e.g. 129105, 132120, 132488, it's not! a dup as it's reg. all filters while the others mention only the behaviour of autofilter, 

someone is working in that area and added '// FIXME this was wrongly exported to the TABLE namespace since 2011' to XMLExportDataPilot.cxx shortly?, would be nice and might also help him if he could have a look at this bug, 

@xisco: if you like to group the bugs together please! let this one as new as it's a clear description, 

let's hurry up and get this fixed before 7.0 release ... 

Steps to Reproduce:
1. fresh sheet B3:a C3:b D3:c, B4:1 C4:2 D4:1, B5:2 C5:1 D5:2, B6:3 C6:1 D6:1, B7:2 C7:2, D7:2
2. data - define range "b" B3:D7
3. data - more filters - standard filter 'a = 2' AND 'b = 1'
4. save file, close file, reload file, save file with new name
5. inspect files with packer - e.g. 7-zip - and look into the contained 'content.xml' file
6. first save: 
------------
<table:database-ranges>
   <table:database-range table:name="b" table:target-range-address="Sheet1.B3:Sheet1.D7">
      <table:filter>
         <table:filter-and>
            <table:filter-condition table:field-number="0" table:data-type="number" table:value="2" table:operator="="/>
            <table:filter-condition table:field-number="1" table:data-type="number" table:value="1" table:operator="="/> 
------------
7. second save: 
------------
<table:database-ranges>
   <table:database-range table:name="b" table:target-range-address="Sheet1.B3:Sheet1.D7" table:orientation="column">
      <table:filter>
         <table:filter-and>
            <table:filter-condition table:field-number="-1" table:data-type="number" table:value="2" table:operator="="/>
            <table:filter-condition table:field-number="0" table:data-type="number" table:value="1" table:operator="="/>
------------
8. observe: 
8a. the additional tag ' table:orientation="column"' for the database range, 
8b. the different values for 'field-number' changed from "0" and "1" in the first save to "-1" and "0" in the second, 
9. the tag 'column' changes the interpretation of the field-number values from 'column-offset' to 'row-offset' and thus the gui works (correct (double incorrect) labels if you reopen the filter definition), but access by basic (macros) fails, 

Actual Results:
deviating definitions on subsequent saves violating the document standards

Expected Results:
identical definitions on subsequent saves holding the document standards, 


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Version: 7.0.0.0.alpha1
Build ID: 6a03b2a54143a9bc0c6d4c7f1...
CPU threads: 8; OS: Linux 4.19; UI render: default; VCL: gtk3; 
Locale: de-DE (en_US.UTF-8); UI: en-US
Calc:
Comment 1 b. 2020-05-24 07:13:12 UTC
Created attachment 161214 [details]
file 'first save' with correct filter definitions
Comment 2 b. 2020-05-24 07:14:06 UTC
Created attachment 161215 [details]
file 'second save' with wrong filter definitions
Comment 3 b. 2020-05-25 08:51:54 UTC
may i ask senior members to raise severity because: 

- produced documents are not odf 1.2 (extended) compatible after second save, 

- there is a small time window to solve the problem with switching to odf 1.3 in LO 7.0 for future cases before! there are many documents in circulation that could get compatibility problems, 

simple reproducer: 

B3:a, B4:1, B5:2
data - define range - name "b", range "B3:B5"
data - autofilter
click button, uncheck '1', 
file save, use odf 1.2 extended format (tools - options - load/save - general)
close file, load file, 
file save, new name, 

check both saved files with https://odfvalidator.org/, the first will pass, the second will fail
Comment 4 Telesto 2020-05-25 19:44:12 UTC
Created attachment 161279 [details]
Step 1 and 2 File
Comment 5 Telesto 2020-05-25 19:55:51 UTC
I'm seeing the "-1" error with https://odfvalidator.org. However, I really don't know anything about this kind of issue...


Adding Regina, the ODF standard specialist
Comment 6 Telesto 2020-05-25 19:56:21 UTC Comment hidden (obsolete)
Comment 7 Regina Henschel 2020-05-25 21:21:27 UTC
Essential part of the description is "should not start at fields A1-B2-C3-D4-E5-F6 ...,"

I think, it is nothing about ODF, but a simple bug in LibreOffice. Apache OpenOffice does not show this error. In LibreOffice I see it already in LibreOffice 3.5.4.2 
Build ID: 165a79a-7059095-e13bb37-fef39a4-9503d18



(In reply to Telesto from comment #6)
> Second attempt.. Adding Regina, the ODF standard specialist
Sometimes other things are more important than Bugzilla.
Comment 8 b. 2020-05-26 06:29:21 UTC
@Regina: 

thanks for checking, 

yes, it's a bug in calc, not in ODF, 

no, a program shouldn't have bugs producing non conformant documents (this bug is affecting functionality too), 

i suspect fixing it might raise some compatibility problems for existing documents? 

would you agree that it's a good idea to fix it for the use of 1.3 format from the beginning? i checked with recent 7.0 in 1.3 extended format, same behaviour as with 1.2
Comment 9 b. 2020-05-28 08:20:46 UTC
@all: 

could you please check if the following patch fixes this bug, it's 'only' a change of the 'preset?' of bByRow from 'false' to 'true' in xmldrani.cxx, sounds logic for me, bByRow 'false' while 'row' is default is somewhat odd, 

i did some tests, looks good, but please! check also affects to other things like sort, horizontal sort, oriental table orientations etc.,  

and if the patch is ok tell me how to place it as a commit? thanks in advance, 

[LO-main-dir]/sc/source/filter/xml/xmldrani.cxx:

107c107,116
<     bByRow(false),
---
>     // bs-edit: the following is an attempt to solve the wrong applied filter conditions on reload of files saved in *.ods format with active filters, 
>     // bs-edit: see tdf #133336, please recheck!!!, affects only some files where the database range has asymmetric start (off from A1-B2-C3-D4-E5-F6 ...), 
>     // bs-edit: there are oddities left, as without this patch the .field values for the filterdescriptor have been wrong from the first reload on, 
>     // bs-edit: they are now still for load of 'prior-patch' 'second and subsequent' saves, i've not yet checked if old files are 'healed' on new save, 
>     // bs-edit: some affected files may be spotted by negative values for .fields, some more by the tag 'table-orientation="column"', but that may be correct for some of them, 
>     // bs-edit: for the gui access it looks as if the tag 'table-orientation="column"' and the wrong .field values cancel each other out as well with or without the patch, 
>     // bs-edit: the patch is intended to apply from ver. 7.0 on, backport to old ver.?, if the patch holds these comments should be removed starting with ver. 9.0, !!FIXME-9.0!!
>     // bs-edit: original was: bByRow(false),
>     // bs-edit: test: 
>     bByRow(true),

sorry if any mistakes, it's new territory for me ... 

reference for checks in the code: if i'm right last correct version was 3.5.1.2, and bug started in 3.5.2.2
Comment 10 b. 2020-05-29 08:54:26 UTC
i tried to submit it as a patch, 

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

i don't know how to proceed, it would be nice if one of the professionals could put it on the right track (check, release, whatever ...) 

add. reference: 

Change-Id: I1dffdef15ac9add1ab4612f4dbc33ad207d9389b
Comment 11 b. 2020-06-05 06:10:40 UTC
the patch didn't hold, it came up that something is unclear about the use of the flag/tag 'table:orientation="column"', described there: 

https://bugs.documentfoundation.org/show_bug.cgi?id=133529

as of now that's the main problem to be solved, thus setting this duplicate,

*** This bug has been marked as a duplicate of bug 133529 ***
Comment 12 Telesto 2020-06-20 15:28:32 UTC
@Buovjaga/@Xisco
Any opinion on the importance of this:
- produced documents are not odf 1.2 (extended) compatible after second save, 

And especially about this -> there is a small time window to solve the problem with switching to odf 1.3 in LO 7.0 for future cases before!
Comment 13 Regina Henschel 2020-06-20 17:53:59 UTC
I don't think, that the reason is something about table:orientation="column".

There had been a change in the kind data fields are indexed (Kohei Yoshida 2011-11-18). This change is not included in AOO. The index was always relative to the data range before. Now it seems to be absolute to the sheet.

Now the relative number minus data range start is written to file. That is
439  mrExport.AddAttribute(XML_NAMESPACE_TABLE, XML_FIELD_NUMBER, OUString::number(rEntry.nField - nFieldStart));
in XMLExportDatabaseRanges.cxx
And on import
417      rEntry.nField = o3tl::saturating_add(nField, nStartPos);
in xmlfilti.cxx

There it seems to be assumed, that the field is absolute in core and relative in file format.

Also Uno has relative index. There an adaption is e.g. in
5402                      rEntry.nField -= nFieldStart;
in cellsuno.cxx

Somewhere seems to be an additional subtraction/addition, which should not be there.

The problem is only with Standard Filter, not with Autofilter.

This is not only a validation error, but breaks interoperability.
Comment 14 b. 2020-06-20 23:53:55 UTC
hello @Regina, thanks for having a look, 

pls. check tdf#133529 which pinpoints the problem somewhat better, 

'absolute in core and relative in file' is intended, afaik, 

imho the problem is 'orientation', in code reflected by 'bByRow', 

it is - and should be? - used by horizontal sort (re-arrange of columns), 

but it is also - and shouldn't be as there are no horizontal filtering functions? (yet) - evaluated for the calculation of the relative '.field' values of all filter descriptions, 

once these are calculated against the 'row-offset' instead of the 'column-offset' of the database-range they become wrong, and if their reference changes in the lifecycle of the file - which can happen by applying a cross-wise sort, or from using different defaults on a fresh constructed file and on read in of a saved file - the use in the file itself is broken, 

> Somewhere seems to be an additional subtraction/addition, which should not be there.

imho it's not that, but calculating against the wrong reference ... (assuming from analysis of the errors that occur with different row/column offsets), 

> The problem is only with Standard Filter, not with Autofilter.

i have seen all filters affected, for autofilter the GUI is 'auto-corrected' (double negative is positive)?, but the .field values are messed up, 

imho setting the default to bByRow = "false" is wrong as i'd read somewhere in the docs 'by Row' is the default, that can be corrected as i tried in https://gerrit.libreoffice.org/c/core/+/95939, it also solves 'illegal negative values' for .field entries in the files, 

the conflict that cross-wise filtering and sort can't be stored in the same tag and thus harm each other is less easy, imho it needs either two independent tags for the orientation of sort and filter, or filter could / should ignore the setting of orientation as there is no horizontal filtering (yet), 

i see little response to this problem so far, i could try to keep tinkering with the code (although that's really not! my talent, nor do i have any reasonable knowledge or 'like' C++) but i'd need: 

- some 'senior advice' if my analysis so far is reasonably accurate, 

- a statement if ignoring table-orientation by filters is correct, or if an additional tag should be 'invented' for its orientation, 

reg. 

b.