Bug 114788 - Crash in 'Form-based filters' when updating filter name, e.g. the 'Or'
Summary: Crash in 'Form-based filters' when updating filter name, e.g. the 'Or'
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
5.1.0.3 release
Hardware: All All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:6.1.0 target:6.0.0.2 target:5.4.5
Keywords: bibisected, bibisectRequest, haveBacktrace, regression
Depends on:
Blocks:
 
Reported: 2018-01-01 03:32 UTC by Howard Johnson
Modified: 2018-01-05 09:39 UTC (History)
5 users (show)

See Also:
Crash report or crash signature: ["svxform::FmFilterModel::ValidateText(svxform::FmFilterItem const*, rtl::OUString&, rtl::OUString&) const"]


Attachments
A simple demonstration database, but probably any database form will do. (10.52 KB, application/vnd.sun.xml.base)
2018-01-01 03:32 UTC, Howard Johnson
Details
bt with debug symbols (6.29 KB, text/plain)
2018-01-01 10:00 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Howard Johnson 2018-01-01 03:32:01 UTC
Created attachment 138779 [details]
A simple demonstration database, but probably any database form will do.

How to demonstrate:

While probably any database will demonstrate this issue, attached is a simple database that can be used.

Open it in LO 6 (or 5.4).

Open the Form named "Table1".

In the record navigator icon bar click the 'Form-based filter' icon, (the little icon with the funnel in front of it).

A small box labeled 'Filter navigator' opens (in front of a filter data table).
It reads "MainForm", and under that a funnel and 'Ok'.

Click once on the word 'Ok' to select it.
(This is used to select the filter line name.  'Ok' is highlighted.)

Click again in the middle of the word 'Ok', to edit it.  A text cursor appears where you click.

Type enter (i.e. the return key).

The system crashes here.

crashreport.libreoffice.org/stats/crash_details/a05db5a7-bcad-4c79-a4a8-59d70810dd9d
Comment 1 Robert Großkopf 2018-01-01 09:18:06 UTC
(In reply to Howard Johnson from comment #0)

> 
> A small box labeled 'Filter navigator' opens (in front of a filter data
> table).
> It reads "MainForm", and under that a funnel and 'Ok'.

Don't know why it is 'Ok' in your system: Here it is 'Or' in every LO-version ...
> 
> Click once on the word 'Ok' to select it.
> (This is used to select the filter line name.  'Ok' is highlighted.)
> 
> Click again in the middle of the word 'Ok', to edit it.  A text cursor
> appears where you click.

Seems this is a new behavior in LO 5.1.0.3. Don't know why it has been added. But LO crashes immediately when editing the filtername.
Up to LO 5.0.5.2 it isn't possible to edit the name here. So I can't get a text cursor and can't reproduce the crash with this (and earlier) versions.

Tested all with OpenSUSE 42.2 64bit rpm Linux.

I will set this one to NEW, keyword "regression" and version to the first which fails here: LO 5.1.0.3
Comment 2 Julien Nabet 2018-01-01 10:00:58 UTC
Created attachment 138784 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated yesterday I could reproduce this.

I attached a bt with symbols.
Comment 3 Julien Nabet 2018-01-01 10:22:53 UTC
Here's the problematic line:
814 FmFormItem* pFormItem = dynamic_cast<FmFormItem*>(pItem->GetParent()->GetParent());
(see https://opengrok.libreoffice.org/xref/core/svx/source/form/filtnav.cxx#814)

Gdb gives:
(gdb) p *(pItem->GetParent()->GetParent())
$4 = {<svxform::FmFilterData> = {_vptr.FmFilterData = 0x7ffff1025a20 <vtable for svxform::FmFilterModel+16>, m_pParent = 0x0, m_aText = ""}, 
  m_aChildren = std::__debug::vector of length 1, capacity 1 = {0x555558bc6fe0}}

so we have here svxform::FmFilterModel

whereas
(gdb) p *(pItem->GetParent())
$7 = {<svxform::FmFilterData> = {_vptr.FmFilterData = 0x7ffff1025b98 <vtable for svxform::FmFormItem+16>, m_pParent = 0x555559847af0, m_aText = "MainForm"}, 
  m_aChildren = std::__debug::vector of length 1, capacity 1 = {0x555559244cb0}}

which shows svxform::FmFormItem, the type that we want.
Comment 4 Julien Nabet 2018-01-01 10:24:50 UTC
I submitted a patch to review here:
https://gerrit.libreoffice.org/#/c/47229/
Comment 5 Howard Johnson 2018-01-01 19:27:44 UTC
OOps, yes you're right, it's "Or", not "Ok".  My bad. 

(12 straight hrs on the computer is too much yesterday I guess.  Eyes are going bad now.)
Comment 6 Julien Nabet 2018-01-03 09:54:36 UTC
Sorry, the patch was wrong.
Comment 7 Lionel Elie Mamane 2018-01-03 13:54:25 UTC
The problem is that the string "Or" (or "MainForm") for that matter should not be editable, like in LibreOffice 5.0.

Also, the _children_ of the "Or" should be editable, but are not. They used to be in LibreOffice 5.0.

Somehow this got inverted.
Comment 8 Julien Nabet 2018-01-03 15:25:33 UTC
This patch helps:
diff --git a/svx/source/form/filtnav.cxx b/svx/source/form/filtnav.cxx
index 37a91bf0d4e3..8dd28703a9d9 100644
--- a/svx/source/form/filtnav.cxx
+++ b/svx/source/form/filtnav.cxx
@@ -1144,7 +1144,7 @@ bool FmFilterNavigator::EditingEntry( SvTreeListEntry* pEntry, Selection& rSelec
     if (!SvTreeListBox::EditingEntry( pEntry, rSelection ))
         return false;
 
-    return pEntry && dynamic_cast<const FmFilterItem*>(static_cast<FmFilterData*>(pEntry->GetUserData())) == nullptr;
+    return pEntry && dynamic_cast<const FmFilterItem*>(static_cast<FmFilterData*>(pEntry->GetUserData())) != nullptr;
 }

=> I can edit a value of the child of "Or"
=> I can't edit "Or" or "MainForm"
The only pb is if I click on the first "Or" or child of "Or" and select "Delete", it indeed deletes it changes the next one from "Or" to "Filter for"
But perhaps we may push this patch and fix the other pb afterwards?
Comment 9 Julien Nabet 2018-01-03 17:20:53 UTC
bt for info:
#0  0x00007ffff0ae2d70 in svxform::FmFilterNavigator::EditingEntry(SvTreeListEntry*, Selection&) (this=0x55555ccfe3f0, pEntry=0x55555c906180, rSelection=...)
    at /home/julien/lo/libreoffice/svx/source/form/filtnav.cxx:1147
#1  0x00007fffee78c396 in SvTreeListBox::ImplEditEntry(SvTreeListEntry*) (this=0x55555ccfe3f0, pEntry=0x55555c906180)
    at /home/julien/lo/libreoffice/svtools/source/contnr/treelistbox.cxx:2548
#2  0x00007fffee78c13f in SvTreeListBox::EditEntry(SvTreeListEntry*) (this=0x55555ccfe3f0, pEntry=0x55555c906180)
    at /home/julien/lo/libreoffice/svtools/source/contnr/treelistbox.cxx:2503
#3  0x00007ffff0ae517e in svxform::FmFilterNavigator::Command(CommandEvent const&) (this=0x55555ccfe3f0, rEvt=...)
    at /home/julien/lo/libreoffice/svx/source/form/filtnav.cxx:1632
Comment 10 Xisco Faulí 2018-01-03 17:40:49 UTC
Regression introduced in range https://cgit.freedesktop.org/libreoffice/core/log/?qt=range&q=4f1dca5083c5a301181786b563b165f19a9dec7f..419549b095a1bb95ce23bf3fc8866e6b582e6dde
Bibisected with lo-linux-dbgutil-daily-till51
Comment 11 Julien Nabet 2018-01-03 17:46:51 UTC
Thank you Xisco, I think it's due to:
https://cgit.freedesktop.org/libreoffice/core/diff/svx/source/form/filtnav.cxx?id=85f93697defd9a812a0cda0bc4e9364e28c0339e

@@ -1193,7 +1193,7 @@ bool FmFilterNavigator::EditingEntry( SvTreeListEntry* pEntry, Selection& rSelec
     if (!SvTreeListBox::EditingEntry( pEntry, rSelection ))
         return false;
 
-    return pEntry && static_cast<FmFilterData*>(pEntry->GetUserData())->ISA(FmFilterItem);
+    return pEntry && dynamic_cast<const FmFilterItem*>(static_cast<FmFilterData*>(pEntry->GetUserData())) == nullptr;

the others ->ISA have been converted to != nullptr
Comment 12 Julien Nabet 2018-01-03 18:12:54 UTC
Here's the new patch submitted on gerrit:
https://gerrit.libreoffice.org/#/c/47338/
Comment 13 Commit Notification 2018-01-03 22:23:47 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

tdf#114788: fix crash in form-based filter

It will be available in 6.1.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 14 Commit Notification 2018-01-04 06:48:00 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=93f8172ffb783da95391ac53ab6402f5a055aa44&h=libreoffice-6-0

tdf#114788: fix crash in form-based filter

It will be available in 6.0.0.2.

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 15 Julien Nabet 2018-01-04 08:38:43 UTC
Waiting for review for patch on 5.4:
https://gerrit.libreoffice.org/#/c/47370/
Comment 16 Commit Notification 2018-01-04 13:31:47 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-5-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3877cd896236b604d0d195260f6c5119f0913c3e&h=libreoffice-5-4

tdf#114788: fix crash in form-based filter

It will be available in 5.4.5.

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 17 Howard Johnson 2018-01-05 09:08:52 UTC
I tested this bug w/ 5.4.5 on linux debian 9.3:

Now when click on 'Or' it only selects it.  No edit and no crash.  

Good.  Excellent.  

(Was interesting to watch the process too.  Thanks!)