Bug 119743 - Libreoffice 6.1 base: can't save special options
Summary: Libreoffice 6.1 base: can't save special options
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.0.3 release
Hardware: All All
: high major
Assignee: Julien Nabet
URL:
Whiteboard: target:6.2.0 target:6.1.3
Keywords: bibisected, bisected, regression
Depends on:
Blocks:
 
Reported: 2018-09-07 09:12 UTC by Daniele
Modified: 2019-03-10 09:23 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniele 2018-09-07 09:12:31 UTC
Description:
I'm using external data sources with JDBC. On Edit->database->advanced settings->special settings you can select the options (https://help.libreoffice.org/Common/Special_Settings), but the changes are not saved on the odb file.

I usually set "Replace named parameters with ?" as true. It's essential for using a H2 database.

I reverted to Libreoffice 6.0.6, and everything works fine.

Steps to Reproduce:
1. Create a database using an external source (JDBC)
2. Change any special setting (https://help.libreoffice.org/Common/Special_Settings), like "Replace named parameters with ?" as true
3. save
4. close the file and open it again

Actual Results:
the special options are not saved

Expected Results:
the special options should be saved


Reproducible: Always


User Profile Reset: Yes



Additional Info:
On Libreoffice 6.0.6 everything works fine.

I usually set "Replace named parameters with ?" as true. It's essential for using a H2 database.
Comment 1 Julien Nabet 2018-09-07 18:15:36 UTC
On pc Debian x86-64 with master sources updated today, I could reproduce this with a brand new odb file embedded hsqldb.

I checked "End text lines with CR+LF", clicked "OK" button.
Then when reopening, the corresponding checkbox was unchecked.
Comment 2 Julien Nabet 2018-09-07 21:12:19 UTC
Seems related to https://cgit.freedesktop.org/libreoffice/core/commit/?id=d6fce54c82868b82bd6fa190db6047d69bbb3ecf

Lionel: reading comment of the commit, why don't we match 2) with 1) and so display only driver/datasource properties supported?

In my case if "End text lines with CR+LF" doesn't stay enabled after having checked the option on my hsqldb embedded database, I suppose it's because hsqldb embedded database doesn't support it. So what's the interest to display the option?

Reading https://cgit.freedesktop.org/libreoffice/core/commit/?id=a29d97e6ddab8ec002ba9827bd5fc874117712e0, I wonder if connectivity/registry/hsqldb/org/openoffice/Office/DataAccess/Drivers.xcu should be changed to replace "Features" by "Properties"

Now about reporter's case and JDBC, I noticed that ParameterNameSubstitution was in Features part. What about putting it a ParameterNameSubstitution block in Properties part but with false value by default?

More simply shouldn't we remove getFeatures and just keep getProperties?
+ some cleanup in Drivers.xcu files
Comment 3 Julien Nabet 2018-09-07 21:20:59 UTC
Forgot to tell that if I do:
diff --git a/dbaccess/source/ui/dlg/DbAdminImpl.cxx b/dbaccess/source/ui/dlg/DbAdminImpl.cxx
index bc0f8b4813ee..50ff50d0c688 100644
--- a/dbaccess/source/ui/dlg/DbAdminImpl.cxx
+++ b/dbaccess/source/ui/dlg/DbAdminImpl.cxx
@@ -699,8 +699,7 @@ void ODbDataSourceAdministrationHelper::fillDatasourceInfo(const SfxItemSet& _rS
     {
         const SfxPoolItem* pCurrent = _rSource.GetItem(static_cast<sal_uInt16>(detailId));
         aTranslation = m_aIndirectPropTranslator.find(detailId);
-        if ( pCurrent && (m_aIndirectPropTranslator.end() != aTranslation) &&
-             aProperties.has(aTranslation->second) )
+        if ( pCurrent && (m_aIndirectPropTranslator.end() != aTranslation) )
         {
             if ( aTranslation->second == INFO_CHARSET )
             {
I could save the option (without changing any other file).
Comment 4 Julien Nabet 2018-09-07 21:46:11 UTC
Trying to understand all this, I noticed that there are:
"PreferDosLikeLineEnds" and "UseDOSLineEnds"
Shouldn't we have only 1 var? What's the diff between them?
For "ParameterNameSubstitution", I don't see any alternative var.
Comment 5 Lionel Elie Mamane 2018-09-13 11:46:46 UTC
It looks like ODbDataSourceAdministrationHelper::fillDatasourceInfo (the function I changed in d6fce54c82868b82bd6fa190db6047d69bbb3ecf) is used not only by the database creation wizard, but also by the "save options" dialog. I missed that when I did that change...

(In reply to Julien Nabet from comment #2)
> Now about reporter's case and JDBC, I noticed that ParameterNameSubstitution
> was in Features part. What about putting it a ParameterNameSubstitution
> block in Properties part but with false value by default?

In my understanding, any Drivers.xcu file which has a _Feature_ without having the corresponding _Property_ is buggy. According to my understanding of what a Feature is and what a Property is, it makes no sense:

 * The Feature says "please allow the user to change this setting"
 * The Property says "this setting is supported by the driver"

So yes, I would change all Drivers.xcu files to add (if not already present) the corresponding Property to each entry in Features. This should solve this bug without further code change.

There is some complexification of that; for most features the corresponding property has the _same_ _name_. But e.g. the "UseBracketedOuterJoinSyntax" feature corresponds to the "EnableOuterJoinEscape" property! That's the only exception known to me out of the top of my head.

Another approach would be to automatically enable the corresponding property of an enabled feature. The current .xcu files seem to have been written in that assumption. It would probably be better backwards-compatible towards external SDBC drivers (that are not bundled with LibreOffice but come e.g. as an extension), but would, I think, be more complicated to do, and leads to less clear and orthogonal semantics of Drivers.xcu files...

(In reply to Julien Nabet from comment #4)
> Trying to understand all this, I noticed that there are:
> "PreferDosLikeLineEnds" and "UseDOSLineEnds"
> Shouldn't we have only 1 var? What's the diff between them?
> For "ParameterNameSubstitution", I don't see any alternative var.

You found a (the?) second exception: The "UseDOSLineEnds" feature corresponds to the "PreferDosLikeLineEnds" property.
Comment 6 Julien Nabet 2018-09-13 19:37:05 UTC
Thank you Lionel!

So I've got to check these 14 files:
./evoab2/org/openoffice/Office/DataAccess/Drivers.xcu
./odbc/org/openoffice/Office/DataAccess/Drivers.xcu
./writer/org/openoffice/Office/DataAccess/Drivers.xcu
./jdbc/org/openoffice/Office/DataAccess/Drivers.xcu
./mysqlc/org/openoffice/Office/DataAccess/Drivers.xcu
./calc/org/openoffice/Office/DataAccess/Drivers.xcu
./flat/org/openoffice/Office/DataAccess/Drivers.xcu
./mork/org/openoffice/Office/DataAccess/Drivers.xcu
./macab/org/openoffice/Office/DataAccess/Drivers.xcu
./dbase/org/openoffice/Office/DataAccess/Drivers.xcu
./hsqldb/org/openoffice/Office/DataAccess/Drivers.xcu
./ado/org/openoffice/Office/DataAccess/Drivers.xcu
./postgresql/org/openoffice/Office/DataAccess/Drivers.xcu
./firebird/org/openoffice/Office/DataAccess/Drivers.xcu
and will submit a patch.
Comment 7 Julien Nabet 2018-09-13 20:42:22 UTC
Another one:
Properties: EnableSQL92Check
Features: UseSQL92NamingConstraints
Comment 8 Julien Nabet 2018-09-13 21:03:09 UTC
Patch submitted on gerrit for review:
https://gerrit.libreoffice.org/#/c/60471/
Comment 9 Xisco Faulí 2018-09-14 09:04:03 UTC
(In reply to Julien Nabet from comment #2)
> Seems related to
> https://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=d6fce54c82868b82bd6fa190db6047d69bbb3ecf

Putting it to bisected then...
Comment 10 Commit Notification 2018-09-20 21:07:36 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

tdf#119743: add Features entries in Properties block Drivers.xcu

It will be available in 6.2.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 11 Commit Notification 2018-10-04 17:48:37 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8d0f2c5192d2c6bc18e406906825c0889e1edc22&h=libreoffice-6-1

tdf#119743: add Features entries in Properties block Drivers.xcu

It will be available in 6.1.3.

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 12 Alex Thurgood 2019-03-06 11:15:19 UTC
@Julien : possible fallout from these fixes in bug 121092 ?
Comment 13 Mike Kaganski 2019-03-10 09:23:54 UTC
(In reply to Lionel Elie Mamane from comment #5)
> In my understanding, any Drivers.xcu file which has a _Feature_ without
> having the corresponding _Property_ is buggy. According to my understanding
> of what a Feature is and what a Property is, it makes no sense:
> 
>  * The Feature says "please allow the user to change this setting"
>  * The Property says "this setting is supported by the driver"

AFAICS, lcl_getFeatureSet in dbaccess/source/ui/misc/dsmeta.cxx gets all things listed under Features, regardless of its Value (true/false). The feature set is used further when deciding which controls are shown on the Advanced dialog; so setting anything in Features to false doesn't hide a control on the dialog.