Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Zolnai Tamás]

Hi Stephan,

>> 2. About your idea how to fix it in windows registry handler code.
>> I hope I understand your words well. So you suggest to have a third
>> entry for Windows registry keys which specifies the type. For the
>> first thought it was a good idea, as you sad it would be easy to
>> implement. However on the second thought, It was not. LO config
>> manager knows the type of a configuration property, it can search the
>> type of a property in LO configuration files. I think this type
>> deduction or type detection is a function of config manager and not a
>> bug or a hack. Your suggestion means that we add the users some extra
>
>
> No, there's no extra work we burden on the user.  A prop of type oor:any
> (which extension props implicitly are) can take on values of different types
> at different times.  Whenever you specify the value of such a prop, you also
> need to specify the type.
>
> (Claiming that this is adding extra burden on the user is a bit like
> claiming that it is extra burden that e.g. for a boolean prop you need to
> specify the value "false" explicitly, instead of guessing that if no value
> is specified then "false" should be implied.)

Well, If we can't agree at least in that it takes more time to specify
the type explicitly in the Windows registry, than don't, then this
conversation might not lead anywhere. C++ language also has a type
deduction (e.g. auto), in spite of programmers must intimately know
what kind of objects they are using in the code.

>> work (specifying the type of a property), which can be handled by the
>> software itself (my change is about this). As I know software
>
>
> No, again, the software cannot guess the missing type in general.  See
> above.

I see. You are thinking in generalities. I would say, yes config
manager can't guess type in general, but it does not mean it can't
guess it in some usefull cases.

So to be more specific. We have an extensible group defined in
officecfg/registry/schema/org/openoffice/Office/Jobs.xcs called
"Arguments". Here there are no properties defined.
The actual properties are defined in
extensions/source/update/check/org/openoffice/Office/Jobs.xcu. Here
the properties are defined with a name and a type.
As I see this xcu file is converted to an xcd file placed in instdir
(onlineupdate.xcd). This xcd file is read by the config manager
and this allows to have a type hint. So type manager does not work
only based on the scheme file (which defines an extensible group wich
properties might have any type). Also the user need to know the actual
"implementation" of an extensible group (which is defined empty in the
scheme file).

It's true that scheme file does not specify the type of properties of
this extensible group, but also it does not specify the properties at
all. A property called "AutoDownloadEnabled" is defined only in the
onlineupdate.xcd (or Jobs.xcu) file which also defines the type of it.
So when a user finds this property in the LibreOffice configuration
files it clearly has a type. I don't think the property called
"AutoDownloadEnabled" can be set to a different typed value (e.g. via
registrymodification.xcu) without making LO behave unexpectedly. (code
handles this property as bool: see cui/source/options/optupdt.cxx).

Summary, you must be right in general, that configuration scheme files
does not allow to guess the type of such properties. I still think it
can be usefull for the users when an extensible group's properties are
well defined in an xcd file, but ok I see this kind of XCU files are
invalid (now that I saw the specification). So the problem is that we
are using this temporary xcu file for applying windows registry
settings. I will have a look at this problem.

Best Regards,
Tamás
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Stephan Bergmann]

On 03/07/2017 02:36 PM, Zolnai Tamás wrote:

1. From the users perspective (XCU user).
What I see is that user can specify a property in an XCU file without
a type if this proprty is not a member of an extensible group. This
bug I'm trying to fix affects only that rare case when this property
is a member of an extensible group. So my first question is that is
this validity concern is only related to that corner case when a
property is a member of an extensible group? Are other kind of
properties OK if they are used without a type in an XCU file? (I'm
speaking such xcu files which are merged with other LO configuration
files (e.g. registrymodification.xcu) and so type is known)


Yes, for xcu prop elements the oor:type attribute is implied.  It 
normally is implied to be the same as the oor:type attribute defined for 
the corresponding schema (xcs) prop element.  Only if the prop is 
defined to be of type oor:any (which is in particular the case for such 
extension props) it must provide an oor:type attribute in xcu.



Is there any specification of these configuration file formats in the meantime?


See the original OOo specification at 
 
that I mentioned earlier, and see the officecfg/registry/*.dtd files.



Other question is that from the user perspective what the difference
between these two kind of properties? (member of an extensible group
or not a member of an extensible group). As a user should I handle
these two kind of properties on a different way? Actually as a user I
would focus on the information that both are properties, both can be
set via Windows registry and I would not care about the
implementation. So I think if allowing this "invalid input" makes
users' every day life easier then I think we should step on this
direction and make this invalid input a valid input.


When you provide such configuration data via the Windows registry, you 
apparently need to intimately know the corresponding xcs schema, just as 
if you provide such configuration data via an .xcu file (via an .oxt 
extension that you wrote, say).  And yes, users who provide such 
configuration data are apparently expected to know what information they 
need to provide for an extension prop.  (Typical end users are not the 
target audience for manually constructed configuration data anyway. 
"Tools - Options... - LibreOffice - Advanced - Open Expert 
Configuration" is the most they're supposed to be exposed to.)


And, in general, there is no way to make invalid input of an extension 
prop value lacking a type valid.  Only if that prop happens to already 
be set by a lower layer could you guess that this instance shall have 
the same type (or not) as the previous one.



2. About your idea how to fix it in windows registry handler code.
I hope I understand your words well. So you suggest to have a third
entry for Windows registry keys which specifies the type. For the
first thought it was a good idea, as you sad it would be easy to
implement. However on the second thought, It was not. LO config
manager knows the type of a configuration property, it can search the
type of a property in LO configuration files. I think this type
deduction or type detection is a function of config manager and not a
bug or a hack. Your suggestion means that we add the users some extra


No, there's no extra work we burden on the user.  A prop of type oor:any 
(which extension props implicitly are) can take on values of different 
types at different times.  Whenever you specify the value of such a 
prop, you also need to specify the type.


(Claiming that this is adding extra burden on the user is a bit like 
claiming that it is extra burden that e.g. for a boolean prop you need 
to specify the value "false" explicitly, instead of guessing that if no 
value is specified then "false" should be implied.)



work (specifying the type of a property), which can be handled by the
software itself (my change is about this). As I know software


No, again, the software cannot guess the missing type in general.  See 
above.



development used to work on the oposite direction with the aim of
making users life easier, make them get the same result with less
effort, right? So that's why I don't think it's a good idea what you
suggest.

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Zolnai Tamás]

Hi Stephan,

So I think this conversation is heading somewhere. So two things I
have in my minds.

1. From the users perspective (XCU user).
What I see is that user can specify a property in an XCU file without
a type if this proprty is not a member of an extensible group. This
bug I'm trying to fix affects only that rare case when this property
is a member of an extensible group. So my first question is that is
this validity concern is only related to that corner case when a
property is a member of an extensible group? Are other kind of
properties OK if they are used without a type in an XCU file? (I'm
speaking such xcu files which are merged with other LO configuration
files (e.g. registrymodification.xcu) and so type is known)
Is there any specification of these configuration file formats in the meantime?
Other question is that from the user perspective what the difference
between these two kind of properties? (member of an extensible group
or not a member of an extensible group). As a user should I handle
these two kind of properties on a different way? Actually as a user I
would focus on the information that both are properties, both can be
set via Windows registry and I would not care about the
implementation. So I think if allowing this "invalid input" makes
users' every day life easier then I think we should step on this
direction and make this invalid input a valid input.

2. About your idea how to fix it in windows registry handler code.
I hope I understand your words well. So you suggest to have a third
entry for Windows registry keys which specifies the type. For the
first thought it was a good idea, as you sad it would be easy to
implement. However on the second thought, It was not. LO config
manager knows the type of a configuration property, it can search the
type of a property in LO configuration files. I think this type
deduction or type detection is a function of config manager and not a
bug or a hack. Your suggestion means that we add the users some extra
work (specifying the type of a property), which can be handled by the
software itself (my change is about this). As I know software
development used to work on the oposite direction with the aim of
making users life easier, make them get the same result with less
effort, right? So that's why I don't think it's a good idea what you
suggest.

Best Regards,
Tamás

2017-03-07 10:30 GMT+01:00 Stephan Bergmann :
> On 03/07/2017 10:00 AM, Zolnai Tamás wrote:
>>>
>>> So coming back to , I'm not
>>> too
>>> excited about that hack.  It's a quick-fix to work around one specific
>>> shortcoming of the naive Windows registry mapping, but in a way that
>>> would
>>> require that hack to stay for backwards-compatibility reasons even
>>> when/if a
>>> more principled fix for that mapping is done.
>>
>>
>> What do you mean by that "would require that hack to stay for
>> backwards-compatibility reasons"? Why? Do you mean that if now we
>
>
> What I meant is:
>
> Encoding an extension prop in the Windows registry is not possible today
> (because the encoding lacks type information).  So a Windows registry
> instance containing an extension prop (necessarily lacking type information)
> is invalid input to LO.
>
> Gerrit 34933 makes valid certain such invalid inputs.  Users will rely on
> this by providing such inputs.
>
> And users will expect that such inputs remain valid in the future.  (Or
> would you be fine with incompatibly forbidding such inputs again in the
> future?)  Which implies that we'll need to keep the code from Gerrit 34933
> around.
>
>> handle a property without an explicit type (inside an extensible
>> group), than this will be required in the future too? As I see config
>
>
> Yes.  If we now give meaning to certain so-far invalid inputs, we'll
> probably need to continue to do so, see above.
>
>> manager works on the same way for other properties which are not
>> member of an extensible group. It's also an invalid thing, right? And
>
>
> I don't understand this part.
>
>> also the temporary xcu file is generated by LibreOffice, so I hope
>> nobody uses this temporarily generated xcu file as a template or
>> something like that.
>
>
> I'm not sure what the temporary xcu files have to do with this.
>
>> Also I think that principle fix you mentioned is not something which
>> would be allowed to push to a bugix branch, so I think it's better to
>> make it work, if users use it. Of course you are right on that this
>> windos registry handling can be improved.
>
>
> What's wrong with the approach of adding a "Type" key?  For its legal values
> use strings, either the "canonically namespace-prefixed" values (cf.
> officecfg/registry/component-update.dtd)
>
>   "oor:any", "xs:boolean", "xs:short", "xs:int", "xs:long", "xs:double",
> "xs:string", "xs:hexBinary", "oor:boolean-list", "oor:short-list",
> "oor:int-list", "oor:long-list", "oor:double-list", "oor:string-list",
> 

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Stephan Bergmann]

On 03/07/2017 10:00 AM, Zolnai Tamás wrote:

So coming back to , I'm not too
excited about that hack.  It's a quick-fix to work around one specific
shortcoming of the naive Windows registry mapping, but in a way that would
require that hack to stay for backwards-compatibility reasons even when/if a
more principled fix for that mapping is done.


What do you mean by that "would require that hack to stay for
backwards-compatibility reasons"? Why? Do you mean that if now we


What I meant is:

Encoding an extension prop in the Windows registry is not possible today 
(because the encoding lacks type information).  So a Windows registry 
instance containing an extension prop (necessarily lacking type 
information) is invalid input to LO.


Gerrit 34933 makes valid certain such invalid inputs.  Users will rely 
on this by providing such inputs.


And users will expect that such inputs remain valid in the future.  (Or 
would you be fine with incompatibly forbidding such inputs again in the 
future?)  Which implies that we'll need to keep the code from Gerrit 
34933 around.



handle a property without an explicit type (inside an extensible
group), than this will be required in the future too? As I see config


Yes.  If we now give meaning to certain so-far invalid inputs, we'll 
probably need to continue to do so, see above.



manager works on the same way for other properties which are not
member of an extensible group. It's also an invalid thing, right? And


I don't understand this part.


also the temporary xcu file is generated by LibreOffice, so I hope
nobody uses this temporarily generated xcu file as a template or
something like that.


I'm not sure what the temporary xcu files have to do with this.


Also I think that principle fix you mentioned is not something which
would be allowed to push to a bugix branch, so I think it's better to
make it work, if users use it. Of course you are right on that this
windos registry handling can be improved.


What's wrong with the approach of adding a "Type" key?  For its legal 
values use strings, either the "canonically namespace-prefixed" values 
(cf. officecfg/registry/component-update.dtd)


  "oor:any", "xs:boolean", "xs:short", "xs:int", "xs:long", 
"xs:double", "xs:string", "xs:hexBinary", "oor:boolean-list", 
"oor:short-list", "oor:int-list", "oor:long-list", "oor:double-list", 
"oor:string-list", "oor:hexBinary-list"


or the raw

  "any", "boolean", "short", "int", "long", "double", "string", 
"hexBinary", "boolean-list", "short-list", "int-list", "long-list", 
"double-list", "string-list", "hexBinary-list"


___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Zolnai Tamás]

Hi Stephan,

> So coming back to , I'm not too
> excited about that hack.  It's a quick-fix to work around one specific
> shortcoming of the naive Windows registry mapping, but in a way that would
> require that hack to stay for backwards-compatibility reasons even when/if a
> more principled fix for that mapping is done.

What do you mean by that "would require that hack to stay for
backwards-compatibility reasons"? Why? Do you mean that if now we
handle a property without an explicit type (inside an extensible
group), than this will be required in the future too? As I see config
manager works on the same way for other properties which are not
member of an extensible group. It's also an invalid thing, right? And
also the temporary xcu file is generated by LibreOffice, so I hope
nobody uses this temporarily generated xcu file as a template or
something like that.
Also I think that principle fix you mentioned is not something which
would be allowed to push to a bugix branch, so I think it's better to
make it work, if users use it. Of course you are right on that this
windos registry handling can be improved.

Best Regards,
Tamás
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Stephan Bergmann]

On 03/06/2017 09:43 PM, Tamas Zolnai wrote:

Anyway, you must be right with that it's not a good idea to change the 
staticType_, so I added an other fix which does not affect staticType_, but 
uses a typeHint only for parsing a specific value from XCU file.
Can you review this change please? I hope you're happy with that:
https://gerrit.libreoffice.org/#/c/34933/


see below


On Monday, March 06, 2017 18:40 GMT, "Tamas Zolnai" 
 wrote:

So it's clear that the generated xcu file from Windows registry is 
incomplete\invalid, so it can be a good idea to fix it, but this would take an 
amount of time since registry keys used for LO are always strings as I see (see 
comments in configmgr/source/winreg.cxx) and I guess it will work on the same 
way for the end of the time for compatibility reasons. So it is impossible to 
find out the type from the registry key. To find out  that we need to parse the 
corresponding xcd file here too (as configmgr code does).


The existing mapping from the Windows registry to the configmgr data 
model is naive, which inevitably gives rise to issues like tdf#106283.


If you/anybody wants to promote that Windows registry feature, it would 
make sense to invest time into a proper mapping between the two data 
models.  (I can help with that and/or you can look at the comment at the 
head of configmgr/source/dconf.cxx for an idea of what to look out 
for---though that mapping is even more complicated as it is 
bidirectional.)  Sure, that may take time (what's the issue with that), 
and may unfortunately be complicated by backwards-compatibility 
concerns.  (An alternative would have been to not allow the existing 
Windows registry feature in in its current naive form, but then again 
the restricted amount of things that can reasonably be done with it in 
its current form may have been sufficient for its initially envisioned 
users.)


(I have only superficial knowledge of the Windows registry, but from 
looking at 
 one 
backwards-compatible way to encode configmgr value+type might be with a 
two-element REG_MULTI_SZ list whose first element encodes the type of 
value in the second element.  Or, even simpler, from looking at the 
comment at the head of configmgr/source/winreg.cxx, in addition to 
"Value" and "Final" keys introduce a "Type" key.  Wouldn't that be 
rather trivial to implement?)



It can be a solution, but as I see you created a bug report (tdf#92755) about avoiding 
using these temporary xcu files for Windows registry, so I'm not sure it worth to try to 
make these xcu files valid (having type information), if they will be removed later, 
right? So I'm not sure why you suggest to fix the code which generates these xcu files. 
(your comment on bugzilla: "[...] For such extension props it must generate an 
oor:type attribute.[...]") Also fixing this bug by replacing xcu generation with a 
better implementation which using configmgr's internal data would also take me very far 
from fixing the bug I intended to (tdf#106283).


Whether or not we go via intermediary temporary xcu files, the mapping 
from the Windows registry to the configmgr data model must be fixed. 
(That is, for such an extension prop, the mapping code needs to provide 
a type that it somehow extracts from the Windows registry data model, 
whether that type is then stored in an intermediary xcu file or directly 
used when instantiation a configmgr::PropertyNode.  That's what I meant 
with "For such extension props it must generate an oor:type attribute.")


Cleaning up those intermediary xcu files is of course orthogonal to 
fixing tdf#106283.



"[...]On the other hand, it is important that the PropertyNode representing
such an extension prop has a staticType_ of TYPE_ANY, so that later layers or
programmatic calls can store values of different type."

So this part is also not clear to me. What do you mean later? When can it 
happen that the same property get a different type, which is defined in the 
specific xcu/xcd file?


: 
"[...] dynamically added properties behave as if defined in the schema 
as nillable, non-localized property of type any."  (And properties of 
type any can take on values of different types over time.)



What do you mean programmatic calls can store values of different type? When a 
programmatic call would store a different typed value for a typed property? One 
specific property always defined with a type even if this property is part of 
an extensible group, right? So I can't see why this type would be overriden by 
a programmatic call? Or if this is a use case (using properties as something in 
which you can store anything) then I guess this also must be true for all 
properties, not only a member of an extensible group. What's the difference?

I also tested that 

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Tamas Zolnai]

Hi Stephan,

Anyway, you must be right with that it's not a good idea to change the 
staticType_, so I added an other fix which does not affect staticType_, but 
uses a typeHint only for parsing a specific value from XCU file.
Can you review this change please? I hope you're happy with that:
https://gerrit.libreoffice.org/#/c/34933/

Thanks,
Tamás 

On Monday, March 06, 2017 18:40 GMT, "Tamas Zolnai" 
<tamas.zol...@collabora.co.uk> wrote: 
 
> Hi Stephan,
> 
> I saw you reverted my change about config manager. Can you help me 
> understanding your comments, please. It's not all clear.
> 
> Your commit message was:
> "Revert "tdf#106283: Registry settings are not read properly on Windows"
> This reverts commit 8cfda7206139b3017346c435591c77c9741ba8ee.  The problem in
> that issue is that the configmgr/source/winreg.cxx code generates .xcu data
> where such an extension prop doesn't have an oor:type attribute, which is not
> allowed.[...]" 
> 
> So it's clear that the generated xcu file from Windows registry is 
> incomplete\invalid, so it can be a good idea to fix it, but this would take 
> an amount of time since registry keys used for LO are always strings as I see 
> (see comments in configmgr/source/winreg.cxx) and I guess it will work on the 
> same way for the end of the time for compatibility reasons. So it is 
> impossible to find out the type from the registry key. To find out  that we 
> need to parse the corresponding xcd file here too (as configmgr code does).
> It can be a solution, but as I see you created a bug report (tdf#92755) about 
> avoiding using these temporary xcu files for Windows registry, so I'm not 
> sure it worth to try to make these xcu files valid (having type information), 
> if they will be removed later, right? So I'm not sure why you suggest to fix 
> the code which generates these xcu files. (your comment on bugzilla: "[...] 
> For such extension props it must generate an oor:type attribute.[...]") Also 
> fixing this bug by replacing xcu generation with a better implementation 
> which using configmgr's internal data would also take me very far from fixing 
> the bug I intended to (tdf#106283).
> 
> "[...]On the other hand, it is important that the PropertyNode representing
> such an extension prop has a staticType_ of TYPE_ANY, so that later layers or
> programmatic calls can store values of different type."
> 
> So this part is also not clear to me. What do you mean later? When can it 
> happen that the same property get a different type, which is defined in the 
> specific xcu/xcd file?
> What do you mean programmatic calls can store values of different type? When 
> a programmatic call would store a different typed value for a typed property? 
> One specific property always defined with a type even if this property is 
> part of an extensible group, right? So I can't see why this type would be 
> overriden by a programmatic call? Or if this is a use case (using properties 
> as something in which you can store anything) then I guess this also must be 
> true for all properties, not only a member of an extensible group. What's the 
> difference?
> 
> I also tested that case when an extensible group has properties with 
> different types (xs:boolean-xs:long) and it also works. I thought you might 
> thinking of that case and later means later when the specific extensible 
> group is extended with a new property with a different type. In this case my 
> change works. So I would appreciate if you can point out a use case when this 
> code change is problematic.
> 
> Thanks,
> Tamás

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: Revert "tdf#106283: Registry settings are not read properly on Windows"

[Tamas Zolnai]

Hi Stephan,

I saw you reverted my change about config manager. Can you help me 
understanding your comments, please. It's not all clear.

Your commit message was:
"Revert "tdf#106283: Registry settings are not read properly on Windows"
This reverts commit 8cfda7206139b3017346c435591c77c9741ba8ee.  The problem in
that issue is that the configmgr/source/winreg.cxx code generates .xcu data
where such an extension prop doesn't have an oor:type attribute, which is not
allowed.[...]" 

So it's clear that the generated xcu file from Windows registry is 
incomplete\invalid, so it can be a good idea to fix it, but this would take an 
amount of time since registry keys used for LO are always strings as I see (see 
comments in configmgr/source/winreg.cxx) and I guess it will work on the same 
way for the end of the time for compatibility reasons. So it is impossible to 
find out the type from the registry key. To find out  that we need to parse the 
corresponding xcd file here too (as configmgr code does).
It can be a solution, but as I see you created a bug report (tdf#92755) about 
avoiding using these temporary xcu files for Windows registry, so I'm not sure 
it worth to try to make these xcu files valid (having type information), if 
they will be removed later, right? So I'm not sure why you suggest to fix the 
code which generates these xcu files. (your comment on bugzilla: "[...] For 
such extension props it must generate an oor:type attribute.[...]") Also fixing 
this bug by replacing xcu generation with a better implementation which using 
configmgr's internal data would also take me very far from fixing the bug I 
intended to (tdf#106283).

"[...]On the other hand, it is important that the PropertyNode representing
such an extension prop has a staticType_ of TYPE_ANY, so that later layers or
programmatic calls can store values of different type."

So this part is also not clear to me. What do you mean later? When can it 
happen that the same property get a different type, which is defined in the 
specific xcu/xcd file?
What do you mean programmatic calls can store values of different type? When a 
programmatic call would store a different typed value for a typed property? One 
specific property always defined with a type even if this property is part of 
an extensible group, right? So I can't see why this type would be overriden by 
a programmatic call? Or if this is a use case (using properties as something in 
which you can store anything) then I guess this also must be true for all 
properties, not only a member of an extensible group. What's the difference?

I also tested that case when an extensible group has properties with different 
types (xs:boolean-xs:long) and it also works. I thought you might thinking of 
that case and later means later when the specific extensible group is extended 
with a new property with a different type. In this case my change works. So I 
would appreciate if you can point out a use case when this code change is 
problematic.

Thanks,
Tamás

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice