Bug Hunting Session
Bug 63416 - FILTER: autofilter -- first row lock
Summary: FILTER: autofilter -- first row lock
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: ux-advise (show other bugs)
Version:
(earliest affected)
3.6.0.0.alpha1
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: BSA target:5.0.0
Keywords:
: 54382 65178 76010 77681 86968 89277 91221 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-04-11 05:26 UTC by ebricca
Modified: 2016-02-22 13:32 UTC (History)
17 users (show)

See Also:
Crash report or crash signature:


Attachments
autofiler example document: before, after, working (8.63 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-04-11 05:26 UTC, ebricca
Details
autofiler example image: after sort asc/desc (2.25 KB, image/png)
2013-04-11 05:32 UTC, ebricca
Details
Code parts responsible for current behaviour (2.54 KB, text/x-c++src)
2015-03-19 07:34 UTC, info
Details
Patch respecting user decision for autofilter header (613 bytes, patch)
2015-03-24 06:15 UTC, info
Details
Screenshot: Autofilter-button remains if header is moved (3.19 KB, image/png)
2015-04-11 17:21 UTC, info
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ebricca 2013-04-11 05:26:24 UTC
Created attachment 77794 [details]
autofiler example document: before, after, working

Problem description: 

first noticed it in 4.0.1.2 rc -- seems to have been worked on but still happening in 4.0.2.2 rc

The autofilter function doesn't work properly if the first row contains an empty field (at first or after edit)
-- first row doesn't stay locked
-- the first complete (data) row is locked instead on the first row

Steps to reproduce:
1. create table with filter header row containing an empty field
2. apply autofilter 
3. use the ascending / descending / empty auto filter sort

(in the example file use filter on "row1" 'sort ascending' then decending to get to the example image)

Current behavior:

filter header row is itself sorted (made invisible) 
-- treated like a data row

if all the fields in the first row are set (also when a sort puts one there) gets locked


Expected behavior:

filter header row should stay locked even if it contains an empty field 

(possibly an option to convert a header row into a data row)


Comments:

about "2. apply autofilter" -- if in 4.0.2.2 the header row contains an empty field a dialog shows up
"first line to be used as column headers"? 
-- but it doesn't have an effect 

when the document is saved/reopened the autofilter is reset to the first row
              
Operating System: Windows 7
Version: 4.0.1.2 release
Comment 1 ebricca 2013-04-11 05:32:12 UTC
Created attachment 77795 [details]
autofiler example image: after sort asc/desc
Comment 2 Joel Madero 2013-04-11 20:29:45 UTC
First, thank you so much for reporting :) I knew that this was an issue but couldn't figure out how to reproduce the issue. 

For the second issue you reported, please make a separate report, even though they are related we try to keep the rule to "1 issue = 1 report"

I have been able to confirm the issue on:
Version 3.6.5.2 
Platform: Bodhi Linux 2.2 x64
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 
As I've been able to confirm this problem on an earlier release I am changing the version number as version is the earliest version that we can confirm the bug, we use comments to say that the bug exists in newer versions as well.

As I've been able to confirm this problem I am marking as:

New (confirmed)
Normal - can prevent high quality and/or professional work
Medium - seems appropriate, easy workaround is "have a header"



Note: I don't think that this is a regression, I tested all the way back to 3.6alpha (from bibisect) and I see the problem if I'm understanding it correctly. Not changing version as I don't do this for bibisect

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
LibreOffice is powered by a team of volunteers, every bug is confirmed (triaged) by human beings who mostly give their time for free. We invite you to join our triaging by checking out this link:
https://wiki.documentfoundation.org/QA/BugTriage and join us on freenode at #libreoffice-qa

There are also other ways to get involved including with marketing, UX, documentation, and of course developing -  http://www.libreoffice.org/get-help/mailing-lists/. 

Lastly, good bug reports help tremendously in making the process go smoother, please always provide reproducible steps (even if it seems easy) and attach any and all relevant material
Comment 3 Björn Michaelsen 2013-04-15 14:36:07 UTC
setting version to 3.6alpha as per last commit, this is not a regression in a microrelease (that is: worked in 3.6.0, didnt in 3.6.Y).
Comment 4 Joel Madero 2013-05-02 20:06:02 UTC
also removing bibisectrequest & regresion - neither confirmed to work. I went through bibisect just to be sure using skip, every time it failed to get correct lock of header with attached file.
Comment 5 Stiftung Buehl 2013-09-06 11:00:58 UTC
This Bug still exit in Version 4.1.1.2-2, not nice!
Comment 6 gibi 2014-02-16 23:45:29 UTC
This annoying bug still exists in 

Version: 4.2.1.1
Build ID: d7dbbd7842e6a58b0f521599204e827654e1fb8b
Comment 7 gibi 2014-02-16 23:54:04 UTC
*** Bug 54382 has been marked as a duplicate of this bug. ***
Comment 8 Kevin Suo 2014-07-07 16:37:56 UTC
*** Bug 76010 has been marked as a duplicate of this bug. ***
Comment 9 Kevin Suo 2014-07-07 16:39:42 UTC
It's worth mentioning that: when sort using the "normal" method (menu "Data - Sort"), it works as expected.
Comment 10 info 2015-03-18 07:24:14 UTC
LibO 4.3.6.2 is affected by this problem.

Is this bug a candidate for the most annoying bug (MAB) list?
Rationale: It is not uncommon that empty columns are used and autofilter is applied.
Comment 11 Joel Madero 2015-03-18 14:55:12 UTC
No - it's not a good candidate. Although it's annoying, it's not equal to loss of data and crashers. Ideally we hope to move away from the MAB list completely and adding these kinds of bugs will just keep that list going. We're going to move to strictly using priority/severity as soon as we can lock these fields from manipulation.

That being said - if you have a suggestion as to how this bug should *objectively*[1] be prioritized, please look at this chart, and give your feedback for what you think and why: 

https://wiki.documentfoundation.org/File:Prioritizing_Bugs_Flowchart.jpg

[1] Please be objective. For instance, if there is a workaround then *by definition* it's a minor bug. Note that this has very little impact on if the bug is actually resolved - if a developer finds a minor or trivial bug annoying, god chance they'll volunteer to fix it, despite the "low" priority/severity setting.
Comment 12 info 2015-03-19 07:34:45 UTC
Created attachment 114181 [details]
Code parts responsible for current behaviour

Thanks for your explanation according to priorities. Since there is a workaround (fill empty column headers with a space) the priority is minor.

I've analysed the code by using the online git view since I do not have a LibO development environment.

Basically, the following lines of code are responsible for the current behaviour:

void ScGridWindow::UpdateAutoFilterFromMenu(AutoFilterMode eMode) {
...
bool bHasHeader = pDoc->HasColHeader(
	aSortParam.nCol1, aSortParam.nRow1, aSortParam.nCol2, aSortParam.nRow2, nTab);

aSortParam.bHasHeader = bHasHeader;

****

bool ScTable::HasColHeader( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW /* nEndRow */ ) const
{
    for (SCCOL nCol=nStartCol; nCol<=nEndCol; nCol++)
    {
        CellType eType = GetCellType( nCol, nStartRow );
        if (eType != CELLTYPE_STRING && eType != CELLTYPE_EDIT)
            return false;
    }
    return true;
}

I've attached the relevant code parts responsible for autofilter sorting, see attachment.

Lacking an IDE, I do not know how often ScTable::HasColHeader is used and what the impact of a change of this method would be.

This autofilter sorting feature was added with commit http://cgit.freedesktop.org/libreoffice/core/commit/?id=2a755c0cd61b619ed14e023ad34fc7596eafdf34

Currently, I do not know what a correct solution would be.

Additionally, there is a warning dialog shown if the autofilter is applied with empty cells in the header: "The range does not contain column headers.
Do you want the first line to be used as column header?" This behaviour should be considered too.
Comment 13 info 2015-03-24 06:15:54 UTC
Created attachment 114291 [details]
Patch respecting user decision for autofilter header

Inbetween I've setup a libreoffice development environment. I've written a little patch, which uses the hasHeader information chosen by the user in ScDBFunc::ToggleAutoFilter().

But does it make sense to have autofilters without headers?
Maybe for filters, but not for sorting. For sorting the autofilter buttons are always moving around.

Should the header in ScGridWindow::UpdateAutoFilterFromMenu() always be set to true?
aSortParam.bHasHeader = true;

Currently the attached patch respecting the user decision (sc_63416_1.diff) has a GUI refresh problem. If the user has chosen to have no headers, the clicked button remains visible. This could be fixed if this patch is the right approach.
Comment 14 Joel Madero 2015-03-25 14:16:00 UTC
Let's ask UX for input on that default behavior.

My sense is that many people do sort without headers (even if that is silly) and that making it default will likely piss a lot of users off ;) But that's not too new so let's see what UX has to say.
Comment 15 info 2015-04-10 19:57:22 UTC
Issue 89277 seems to be a duplicate. Should it be marked as duplicate?
Comment 16 info 2015-04-11 09:30:54 UTC
*** Bug 89277 has been marked as a duplicate of this bug. ***
Comment 17 Yan Pas 2015-04-11 09:38:49 UTC
In bug 89277 headers containing numbers were sorted too. So everything that is no text will not be recognized as header and thus sorted
Comment 18 info 2015-04-11 17:21:02 UTC
Created attachment 114741 [details]
Screenshot: Autofilter-button remains  if header is moved

I've noticed with the current master (f8976c4) the clicked autofilter button remains sometimes after the header is moved, see screenshot. This does not happen with version 4.3.6.2.

How to reproduce:
1. At least 3 Columns, in the middle an empty one, see screenshot.
2. Add autofilter on first column
3. Sort asc and desc with autofilter.
4a. The clicked button remains sometimes.
4b. Repeat step 3 if problem does not occur.

I think this behaviour is not a cause of this bug report, it just a new secondary effect of moving autofilter buttons.

This behaviour appears also if the proposed solution of comment 13 is applied.

Thus, I propose to submit a patch to gerrit for review, as proposed in comment 13. This patch is probably not the final solution, but it is a big improvement. The user decision of having a header or not is respected. It will be far better than the current behaviour.

If this small patch would be committed, another bug report could be written for the GUI update problem. This GUI issue could be solved independently.

Do you agree with my proposition?
Comment 19 info 2015-04-11 19:17:15 UTC
*** Bug 86968 has been marked as a duplicate of this bug. ***
Comment 20 info 2015-04-11 19:27:40 UTC
*** Bug 41781 has been marked as a duplicate of this bug. ***
Comment 21 info 2015-04-12 07:27:07 UTC
Proposition for new behaviour:

1. If there is a header row (detect with pDBData->HasHeader())
    * do not sort the first row (aSortParam.bHasHeader = true)
2. else if there is no header row
    * first row will be sorted too (aSortParam.bHasHeader = false), but autofilter buttons should stay at the first row

Currently, the autofilter buttons will be moved too, if there is no header row. In large files, the autofilter buttons disappear somewhere in the sheet. This is not good.

In the second case (no header row), I propose to change the code as follows
a. Remove autofilter buttons
b. Sort rows
c. Add autofilter buttons again to the new first row.

I think this would be the correct solution. It would meet the expectations of the users.

What do you think about my proposition?
Is moving of the first row necessary? (comment 13)
How can autofilter buttons be removed/added in a correct way?

Additionally, I propose to enhance the Test::testAutofilter() in ucalc.cxx:
1. non-string column headers (empty cells and numbers)
2. with and without header row
Comment 22 Yan Pas 2015-04-12 09:14:08 UTC
I want to add something. If the first row contains numbers, but has cell-type "text" it should not be sorted. For example I have a journal of students, where the first row looks like: Name, Subgroup, 1 (week number), 2 (week number), 3, 4... And below the week numbers are marks. I do not want week numbers were mixed with marks.

And if the first row contains any empty cell it should not be sorted too.
Comment 23 info 2015-04-12 09:29:44 UTC
I assumed that the "Do you want the first line to be used as column header?" dialog remains. That's why the user has the choice. The result of this dialog is then asked with pDBData->HasHeader(). See also bug 65178.

Thus, your case should be handled fine, if you set the header row with the dialog.

Sorry, my proposition was not detailed enough.
Comment 24 Yan Pas 2015-04-12 09:35:35 UTC
This dialog doesn't work :) Whatever I choose all data will be mixed with header
Comment 25 info 2015-04-12 12:14:18 UTC
> This dialog doesn't work :) Whatever I choose all data will be mixed with header

Because it's not yet fixed. It will be respected with the patch of attachment 114291 [details]. Locally, on my computer it works and this decision is respected. But this patch is not enough for a full solution without a header row (i.e. first line contains also data), since the autofilter buttons move with the data line. Thus, my proposition for a new behaviour in comment 21.

In comment 18, I proposed to commit this little patch. Then we gain already a lot, even if the second case (no header, i.e. first row with data), does not work properly. But I assume, most of the time we have a header row. (80-20-rule)
Comment 26 info 2015-04-13 09:33:42 UTC
*** Bug 65178 has been marked as a duplicate of this bug. ***
Comment 27 info 2015-04-13 09:41:01 UTC
I've submitted a patch for review: https://gerrit.libreoffice.org/#/c/15277/
Hopefully, it will be accepted by the reviewers.
Comment 28 info 2015-04-14 17:13:08 UTC
I've compared autofilters with Excel 2010. There is always a header row, i.e. only data rows without header are not possible. Thus, the second case of comment 21 does not exist in Excel 2010.
Comment 29 Heiko Tietze 2015-04-15 21:41:04 UTC
To put it all together:

1. First row with text only
-> make it a header, do not sort, do not hide on filter

2. First row with empty cells only
-> make it a header using 'col XY' or the like, do not sort, do not hide on filter

3. First row with mixed content (text, numbers, and empty cells)
-> make it a header, (temporarily) convert numbers into text, fill empty cells with col XY or the like, do not filter, do not sort

4. First row with numbers only
-> make it a header, (temporarily) convert numbers into text, do not sort, do not filter

Alternatively we could 
a) keep the current behavior, 
b) just add a first row, 
c) disable sort/filter if no header is present, 
d) move the autofilter stuff to the table header (column 'A' would be 'A' with a dropdown menu), or 
e) have the autofilter stuff somewhere else e.g. at the sidebar. 
But for consistency I'd use the same behavior in all cases, probably like Excel does (haven't one at hand).

The question whether or not the first row should become the header would be obsolete.
Comment 30 mahfiaz 2015-04-16 07:58:17 UTC
I think I belong more to the info@scito.ch camp with smart behavior. Which means ux-advise probably needs to talk this through next Wednesday.
Comment 31 Heiko Tietze 2015-04-22 18:38:19 UTC
Postponed to next week
Comment 32 Commit Notification 2015-04-30 12:20:18 UTC
scito committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=00836605a441b8b1d548d8c32f63535f3240ac61

tdf#63416 Do not sort header in autofilter if user has chosen one in dialog

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 33 Yan Pas 2015-09-13 20:32:29 UTC
tested on 5.0.1.2, seems to work fine. Someone test please too. Tommorow I will perform more tests and close as fixed
Comment 34 Yan Pas 2015-09-14 21:11:02 UTC
tested on linux and windows 7 v 5.0.1. Everything seems to work fine. Formats, empty headers, numeric headers, special symbol headers - all works fine.
Comment 35 Cor Nouws 2016-02-22 13:30:06 UTC
@Joel: seeing the commit in comment #32, fixed is proper here, I think?
Comment 36 Cor Nouws 2016-02-22 13:31:35 UTC
*** Bug 91221 has been marked as a duplicate of this bug. ***
Comment 37 Cor Nouws 2016-02-22 13:32:12 UTC
*** Bug 77681 has been marked as a duplicate of this bug. ***