Bug 87840 - checkboxes for not-supported-by-driver advanced settings are present (but have no effect)
Summary: checkboxes for not-supported-by-driver advanced settings are present (but hav...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.3.5.2 release
Hardware: x86 (IA32) All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.5.0 target:4.3.6 target:4.4...
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-29 21:02 UTC by Andrzej Sygiel
Modified: 2015-01-07 23:26 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
advanced connection properties (34.25 KB, image/png)
2014-12-29 21:02 UTC, Andrzej Sygiel
Details
HSQLDB embedded advanced/special dialog on 3.5 (4.96 KB, image/png)
2015-01-06 04:00 UTC, Lionel Elie Mamane
Details
HSQLDB embedded advanced/special dialog on master (4.5.alpha) (10.60 KB, image/png)
2015-01-06 04:01 UTC, Lionel Elie Mamane
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrzej Sygiel 2014-12-29 21:02:49 UTC
Created attachment 111488 [details]
advanced connection properties

There is a problem with calling a query with aliases defined for libreoffice base (odb) connected to ms acces (mdb). I have discovered a strange behaviour of one of the checkboxes on "advanced connection properties" tab. Checkbox named "Use keyword AS before table alias names" despite beeing checked after exit the tab remains unchecked.
Comment 1 raal 2014-12-30 07:34:27 UTC
I can confirm with Version: 4.5.0.0.alpha0+
Build ID: 7f476fea47f06a7f8cc961dd4f6595a524346fa5
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:master, Time: 2014-12-27_23:36:28

I can confirm with LO 4.3.5.2, win7

Also doesn't work on new file
-create new hsqldb database
- right click -> database -> Advanced settings
- tick "Use keyword AS before table alias names"; OK
- right click -> database -> Advanced settings

Actual results:
checkbox "Use keyword AS before table alias names" unticked
Comment 2 Alex Thurgood 2014-12-31 11:25:25 UTC
Adding Julie, Lionel to cc
Comment 3 Alex Thurgood 2014-12-31 11:25:51 UTC
(In reply to Alex Thurgood from comment #2)
> Adding Julie, Lionel to cc

Julien
Comment 4 Julien Nabet 2014-12-31 11:45:11 UTC
I could reproduce this on pc Debian x86-64 with master sources updated today.
The problem seems more general since the bug is for every checkbox of Advance settings.
Comment 5 Julien Nabet 2014-12-31 12:49:00 UTC
After some gdb, it seems it depends on the type of DB.
For hsqldb, the only checkbox which work are:
- End text lines with CR+LF 
- Ignore currency field information
- Use ODBC conformant date/time literals

For thunderbird/Icedove Address book the only working one is:
- Use ODBC conformant date/time literals

With Firebird odbc, every checkbox works except:
- Form data input checks for required fields

BTW, I noticed these lines:
159 {&m_pIgnoreCurrency, "inputchecks", DSID_IGNORECURRENCY, false },
160 {&m_pCheckRequiredFields, "ignorecurrency", DSID_CHECK_REQUIRED_FIELDS, false },
See http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/advancedsettings.cxx#156
it seems there's a mix between line 159 and 160
I'm gonna fix this.

For the rest, if that depends on the type of DB, it's more a kind of UI bug.
Lionel: any thoughts?
Comment 6 Commit Notification 2014-12-31 12:59:55 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Related fdo#87840: mix between ignorecurrency and inputchecks

It will be available in 4.5.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 7 Lionel Elie Mamane 2014-12-31 14:38:54 UTC
(In reply to Julien Nabet from comment #5)
> For the rest, if that depends on the type of DB, it's more a kind of UI bug.
> Lionel: any thoughts?

Possibly the bug is that the checkboxes that are not supposed to be enabled are enabled. Each driver specifies which advanced properties it supports (with default value), and which ones should be exposed in the UI for the user to change. If an advanced property is "supported" but not exposed to the user in the UI, then for most practical purposes it is hardcoded.

See the .xcu files in the source tree in connectivity/registry/*/org/openoffice/Office/DataAccess/Drivers.xcu

The "Properties" node is the _supported_ properties and the "Features" node is the properties that are supposed to be exposed in the UI.
Comment 8 Julien Nabet 2014-12-31 16:50:46 UTC
Thank you Lionel.
Taking a look to these files, I noticed that hsqldb indicates 4 features:
- UseDOSLineEnds
- FormsCheckRequiredFields
- EscapeDateTime
- AddIndexAppendix
See http://opengrok.libreoffice.org/xref/core/connectivity/registry/hsqldb/org/openoffice/Office/DataAccess/Drivers.xcu#28
Except that
"lcl_getFeatureMappings" doesn't mention "AddIndexAppendix"
60         static const FeatureMapping s_aMappings[] = {
61             { DSID_AUTORETRIEVEENABLED,     "GeneratedValues" },
62             { DSID_AUTOINCREMENTVALUE,      "GeneratedValues" },
63             { DSID_AUTORETRIEVEVALUE,       "GeneratedValues" },
64             { DSID_SQL92CHECK,              "UseSQL92NamingConstraints" },
65             { DSID_APPEND_TABLE_ALIAS,      "AppendTableAliasInSelect" },
66             { DSID_AS_BEFORE_CORRNAME,      "UseKeywordAsBeforeAlias" },
67             { DSID_ENABLEOUTERJOIN,         "UseBracketedOuterJoinSyntax" },
68             { DSID_IGNOREDRIVER_PRIV,       "IgnoreDriverPrivileges" },
69             { DSID_PARAMETERNAMESUBST,      "ParameterNameSubstitution" },
70             { DSID_SUPPRESSVERSIONCL,       "DisplayVersionColumns" },
71             { DSID_CATALOG,                 "UseCatalogInSelect" },
72             { DSID_SCHEMA,                  "UseSchemaInSelect" },
73             { DSID_INDEXAPPENDIX,           "UseIndexDirectionKeyword" },
74             { DSID_DOSLINEENDS,             "UseDOSLineEnds" },
75             { DSID_BOOLEANCOMPARISON,       "BooleanComparisonMode" },
76             { DSID_CHECK_REQUIRED_FIELDS,   "FormsCheckRequiredFields" },
77             { DSID_IGNORECURRENCY,          "IgnoreCurrency" },
78             { DSID_ESCAPE_DATETIME,         "EscapeDateTime" },
79             { DSID_PRIMARY_KEY_SUPPORT,     "PrimaryKeySupport" },
80             { DSID_RESPECTRESULTSETTYPE,    "RespectDriverResultSetType" },
81             { DSID_MAX_ROW_SCAN,            "MaxRowScan" },
82             { 0, NULL }
See http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/misc/dsmeta.cxx#58
Moreover, there's no properties in hsqldb file!

I thought about comparing all the Drivers.xcu to:
- check that every one has some properties
- check that Features content contains only known id (DSID...)
Lionel: do you think about additional check or have some remark?
Should I create a new bugtracker to not pollute this one?
Comment 9 Alex Thurgood 2015-01-03 17:41:30 UTC
Adding self to CC if not already on
Comment 10 Commit Notification 2015-01-05 20:44:28 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bda665c442c5e59f5d6656a7d3e54ed1dfdc1b01&h=libreoffice-4-3

Related fdo#87840: mix between ignorecurrency and inputchecks

It will be available in 4.3.6.

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 2015-01-05 20:44:34 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2077e89fd1ba7cae10f22f751a674076b54bf9ab&h=libreoffice-4-4

Related fdo#87840: mix between ignorecurrency and inputchecks

It will be available in 4.4.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 12 Lionel Elie Mamane 2015-01-06 03:09:11 UTC
(In reply to Julien Nabet from comment #8)

> Taking a look to these files, I noticed that hsqldb indicates 4 features:
> - UseDOSLineEnds
> - FormsCheckRequiredFields
> - EscapeDateTime
> - AddIndexAppendix

> Except that
> "lcl_getFeatureMappings" doesn't mention "AddIndexAppendix"

Note also that unfortunately sometimes the feature and the property don't have the same name (!). For example property "EnableOuterJoinEscape" is feature "UseBracketedOuterJoinSyntax" (see comments in connectivity/registry/postgresql/org/openoffice/Office/DataAccess/Drivers.xcu).

So actually, lcl_getFeatureMappings may not do what you think it does. It may actually be (but that's just a conjecture) the mapping from DSID constants to feature names *only* when the name differs from the property?

See for "EnableOuterJoinEscape" / "UseBracketedOuterJoinSyntax":
http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/DbAdminImpl.cxx#182

Similarly, see for "AddIndexAppendix" see
http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/DbAdminImpl.cxx#185
which links DSID_INDEXAPPENDIX to "AddIndexAppendix" (I think as a *property* *name*).

But lcl_getFeatureMappings has DSID_INDEXAPPENDIX linked to "UseIndexDirectionKeyword". Does that mean that the property in connectivity/registry/hsqldb/org/openoffice/Office/DataAccess/Drivers.xcu should be called "UseIndexDirectionKeyword"? Not sure. Try changing it, does it make a difference?

> Moreover, there's no properties in hsqldb file!

Possibly "properties" is not "the ones supported", but "the ones that should have a different default value (for connections through this driver) than the Base-wide default". Possibly HSQLDB does not need to change any default. It could be worth checking that.

OTOH, I notice that connectivity/registry/ado/org/openoffice/Office/DataAccess/Drivers.xcu has some stuff in "MetaData" (e.g. "ColumnAliasInOrderBy") that I would have expected in "Properties" and that is indeed in "Properties" elsewhere...

> I thought about comparing all the Drivers.xcu to:
> - check that every one has some properties

As per above, not having any property is maybe not an error. I'm not sure.

> - check that Features content contains only known id (DSID...)

See above
Comment 13 Lionel Elie Mamane 2015-01-06 03:14:11 UTC
(In reply to Lionel Elie Mamane from comment #7)
> (In reply to Julien Nabet from comment #5)
>> For the rest, if that depends on the type of DB, it's more a kind of UI bug.
>> Lionel: any thoughts?
 
> Possibly the bug is that the checkboxes that are not supposed to be enabled
> are enabled.

Yes, that's most definitely the case. Compare the "Advanced Settings" dialog for an embedded HSQLDB in master (16 checkboxes) and in 3.5.4.2 (three checkboxes).
Comment 14 Lionel Elie Mamane 2015-01-06 03:50:40 UTC
My best guess is that this bug was introduced by the conversion of the "advanced settings" dialog to .ui format. Caolán, could you please help us on this one? Thanks in advance.

The "old" .src code had (file dbaccess/source/ui/dlg/advancedsettings.cxx function SpecialSettingsPage::SpecialSettingsPage):

         if ( rFeatures.has( nItemId ) )
         {
             sal_uInt16 nResourceId = setting->nControlResId;
             (*setting->ppControl) = new CheckBox( this, ModuleRes( nResourceId ) );


which to me looks like that when (! rFeatures.has( nItemId )), the checkbox was never created. The new .ui code has:

            if ( rFeatures.has( nItemId ) )
            {
                get((*setting->ppControl), setting->sControlId);

which (from the name "get" and not "create") smells like the the checkbox is just not put in "*setting->ppControl", but it is actually created.

Caolán, is my guess right?

In this case, how can we "not create" (or hide) checkboxes that we do not want? Note that placement of the checkboxes on the page depends on which ones are wanted, obviously. How do we handle that in .ui format?

Alternatively, we could create and show all checkboxes, but disable (grey out) the ones we do not want. Something like:

            if ( rFeatures.has( nItemId ) )
            {
                get((*setting->ppControl), setting->sControlId);
                (...)
            }
            else
            {
                CheckBox*  pControl;
                get(pControl, setting->sControlId);
                pControl->setEnabled(false);
            }
Comment 15 Lionel Elie Mamane 2015-01-06 04:00:44 UTC
Created attachment 111810 [details]
HSQLDB embedded advanced/special dialog on 3.5
Comment 16 Lionel Elie Mamane 2015-01-06 04:01:25 UTC
Created attachment 111811 [details]
HSQLDB embedded advanced/special dialog on master (4.5.alpha)
Comment 17 Commit Notification 2015-01-07 09:57:43 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

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

Resolves: fdo#87840 by default hide all feature checkboxes

It will be available in 4.5.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 18 Caolán McNamara 2015-01-07 10:10:06 UTC
presumably we can close this now. 4-4 and 4-3 backports of above fix in gerrit
Comment 19 Lionel Elie Mamane 2015-01-07 10:58:10 UTC
(In reply to Caolán McNamara from comment #18)
> presumably we can close this now.

Yes, thanks.

Julien, if you want to address the small "side problems" that you found (like e.g. the feature being not named right in the hdsqldb Drivers.xcu), you still can :)
Comment 20 Commit Notification 2015-01-07 11:03:10 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=db3ab19f6ca3252141f7e7f858fdde406a890cec&h=libreoffice-4-3

Resolves: fdo#87840 by default hide all feature checkboxes

It will be available in 4.3.6.

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 21 Commit Notification 2015-01-07 11:03:16 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=92d5ca9aa6f585de51f7fcfdf756f3bdf91052dc&h=libreoffice-4-4

Resolves: fdo#87840 by default hide all feature checkboxes

It will be available in 4.4.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 22 Julien Nabet 2015-01-07 23:26:31 UTC
Lionel: thank you for your feedback. This part seems quite a mess or at least, not obvious. I prefer letting it as it is instead of risking to add some bugs.