|
Apache OpenOffice (AOO) Bugzilla – Full Text Issue Listing |
|
Description
oc
2004-06-28 14:19:10 UTC
Created attachment 16156 [details]
bugdoc
accepted title adjusted Fix will include import from Excel and export to Excel. *** Issue 37370 has been marked as a duplicate of this issue. *** planned for OOo 3.0 *** Issue 57857 has been marked as a duplicate of this issue. *** Created attachment 35013 [details]
more complex use-case
the attached use case shows the problems that have been explained in issue 57857 very well. When the fix is in place, this file can be used to check the functionality: - open in calc - save as .ods or .xls - close and reopen -> Risk values at the summary sheet must not change target change target from 2.x to 3.x according to http://wiki.services.openoffice.org/wiki/Target_3x Created attachment 54689 [details]
Patch which adds a GroupName property to Radio Buttons for forms controls
Created attachment 54690 [details]
Patch which sets the GroupName property when importing .xls files.
The forms-radio-button-group-names.diff file also patches Toolkit so that the Toolkit Radio Button gets a Group Name property. mt, cd: could you please review the patch to make sure that it's sensible for toolkit? Also, it currently doesn't save the GroupName property for toolkit controls, only forms controls. Do you know what would be involved in adding full toolkit support? dr-> xl-import-formradiobutton.diff is as the name suggests just import *only*. It depends on the presence of forms-radio-button-group-names.diff that jpryor attached ( we thought it made sense to group the 2 patches under this bug, hope that is ok). Although I didn't get a chance to do the export, an export based on the records imported *will* work. Or at least it worked when I experimented with a hardcoded export of 3 groups, a default group and 2 groups bounded by group boxes[*] [*] it seems that the binary format of the radio groups e.g. where the grouping is specified by the pseudo linked list of object id(s) see the import ( 'mnNextRBInGroup' etc. for details ) really does need the additional presence group boxes for the grouping behaviour to be realized ( except for the default group of course ) in MSO Excel. But as you know radio button(s) in openoffice resemble more closely the ole variant of the radio button then the form button control, what exactly to export is a little unclear as well. noel: yes, grouping the patches here is ok :-) and you are right, the olf-fashioned form radiobutton controls that we export do not contain an explicit grouping property, but are automatically grouped as you found out. I have sztarted an experimental export filter for OLE form controls in the past but it didn't work for an unknown reason, therefore we export the other type of controls. See the EXC_EXP_OCX_CTRL define in sc/source/filter/inc/xlocx.hxx if you want to play ;-) daniel:
>See the EXC_EXP_OCX_CTRL define in
>sc/source/filter/inc/xlocx.hxx if you want to play ;-)
yes I have seen that code and always meant to poke you as to its status,
unfortunately at the moment I don't have time to look :-( but it is on my list
of things I want to get to :-) I guess it mostly makes sense in a round trip
scenario ( and/or possible for radio buttons depending on the ref value ).
Anyway hopefully in the not to distant future I might just get a chance play
with that ( I hope I can pick your brains when that time comes )
sure :) a couple of things, 1. Frank already ok'ed jpryor's changes from ( froms-radio-button-group-names.diff ) see http://dba.openoffice.org/servlets/ReadMsg?list=dev&msgNo=3477 and associated thread 2. As jpryor mentioned there is no persistence of the new toolkit GroupName attribute, e.g. if you set the groupname propery in a basic dialog the groupname will be lost 3. There is an awful hack in the XL userform import to apply radio button grouping it should be removed I can provide a patch for 2 & 3 above ( I propose to just use the same attribute name ( with the dialog namespace ) e.g. dlg:groupname ) hope that is ok, I will attach the patch here when done oops that is dlg:group-name Created attachment 54727 [details]
patch file, allows groupname to be pesisted for radiobuttons on basic dialog, also tweaks excel importer
Created attachment 54885 [details]
patch that superceeds the previous version of the same name, it seems I didn't see I could access the ObjectManager from XclRoot, this patch removes the unnecessar passing around of the XclImpObjectManage instance
DR->FS: please review the core parts of the patch. DR->NPOWER: currently I am working in CWS dr63. I have done lots of changes in Excel form controls import (basically I have added complete import of BIFF5 drawing layer including controls). I will take your patch and add your changes manually. To be clear, please confirm: - "forms-radio-button-group-names.diff" adds core functionality and OCX import - "dialog-groupname-persist.diff" - adds dialog support and OCX import too - "xl-import-formradiobutton.diff" adds import for old Excel controls. I see a problem with first two patches, as both are changing OCX import (msocximex.* in SVX) issue type -> PATCH daniel: >I have done lots of changes in Excel form controls import (basically I have added complete import of BIFF5 drawing layer including controls). Oh no, you mean I have to learn how it all works again ;-) >To be clear, please confirm: >- "forms-radio-button-group-names.diff" adds core functionality and OCX import yes, note: the OCX import part is trivial and just applies a previously imported ( but unused ) item ( and should be the only additional part to the patch that Frank has already reviewed, but he would be better to comment on what he has reviewed or not, I didn't follow the thread linked above, just provided some info ) >- "dialog-groupname-persist.diff" - adds dialog support and OCX import too yes >- "xl-import-formradiobutton.diff" adds import for old Excel controls. also yes, and *no* export ( a pity ) but imo still improves the status quo and doesn't make anything worse then it was before. > I see a problem with first two patches, as both are changing OCX import (msocximex.* in SVX) because this is the code you have reorganised/moved/changed ? I didn't look at the cws but if you want me to help with some of those changes I would try ( but might take a couple of days... very busy at the moment :-( ) But I can at the very least I could answer any questions etc. fs->jpryor: The patch is still missing (IIRC, this was one of the last remaining issues in our discussion in dev@dba) the *import* of the property from ODF. It is properly exported, but there's no code for importing it. If I'm not wrong, this should only be a matter of adding a new map entry in OFormLayerXMLImport_Impl::OFormLayerXMLImport_Impl. Other than that, the patch looks like what we discussed in dev@dba. Great stuff, I am looking forward to see this included. Dr->NPOWER: No, the only patch that cannot be applied anymore, is "xl-import-formradiobutton.diff" (changing sc/source/filter/excel/xiescher.cxx). The OCX import part located in SVX is untouched. The problem I mentioned and see is that if we apply both patches "forms-radio-button-group-names.diff" and "dialog-groupname-persist.diff", there may be a conflict, as both patches are changing the same OCX code, don't they? >The problem I mentioned and see is that if we apply both patches
>"forms-radio-button-group-names.diff" and "dialog-groupname-persist.diff", there
>may be a conflict, as both patches are changing the same OCX code, don't they?
there shouldn't be a conflict, dialog-groupname-persist.diff modifies the code
previously added by forms-radio-button-group-names.diff ( it depends on it if
you like ) it they are applied in the order
forms-radio-button-group-names.diff
xl-import-formradiobutton.diff
dialog-groupname-persist.diff
they *should* apply
DR->NPOWER: ok, thanks, so we are waiting for an answer for FS's question... Oops. Apparently I completely forgot about import, and/or thought that it was already in there. :-( I'll implement this as soon as I can and attach an updated patch. Created attachment 55617 [details]
Patch which supersedes the previous patch of the same name, adding form import for the group names.
fs->jpryor: thanks for the update. I hear the proposal for the file format change (for the new group-name attribute), well, had its difficulties passing the OASIS TC recently. This, sadly (I want this feature!), kind of hinders us at the moment to integrate the patch as-is. Do you (your colleagues, that is) plan to submit a reworked proposal, with another (better) justification why the change is needed? Created attachment 56097 [details]
latest version, superceeds previious version
fs->jpryor: ping. Could you please give me an update on your file format plans for this change (see above)? Thanks. >fs->jpryor: ping. Could you please give me an update on your file format plans
>for this change (see above)? Thanks.
I was wonder too and wondering why there was no answer, seem jpryor was not on
the cc ;-) so I added him.
Jon, any idea, I add florian too, can you give him the details too?
> I was wonder too and wondering why there was no answer, seem jpryor was not
> on the cc ;-) so I added him.
*blush* Sorry, I overlooked this, and thanks.
jpryor->fs: I spoke with Florian, and he's as clueless as I about why the proposal was rejected. Apparently it's about accessibility, as that's the subcommittee that stopped the proposal, but beyond that, we have no idea... Is there any chance you could figure out what they didn't like about the proposal, or possibly resubmit it on our behalf? fs->jpryor: Talked with some TC members who participated in the discussions around Florian's proposal. I have been convinced that any new proposal we submit only has chances if we have *very* good arguments why ODF needs this additional attribute. This is difficult because of two facts: a) Other W3C standards (HTML comes to mind immediately) don't use dedicated attributes for grouping, but the name only, so adding a group name attribute to get consistency with MSO, this way sacrificing consistency with other W3C standards (which ODF strives to adhere to where possible) will be ... challenging to argue (at least) to this particular audience. b) Since the proposal had a first round already, which went somewhat ... unfortunate for reasons out of the scope of this issue, any subsequent round in the TC will have an even higher level of attention, so our arguments need to be even better. I don't have a good idea at the moment how to proceed. I don't think /me simply re-submitting the proposal changes anything, we would need to convince the TC of the change (and this requires new arguments, anyway, since A11Y certainly is not involved with this feature at all). Need to think about the matter. If the problem is the attribute, could we instead use a nested element? Would that be as problematic for the TC? I'm also not entirely convinced by their argument. Let's look at XHTML: http://www.w3.org/TR/html/DTD/xhtml1-strict.dtd Radio buttons are done with <input/>: <!ELEMENT input EMPTY> <!-- form control --> <!ATTLIST input %attrs; %focus; type %InputType; "text" name CDATA #IMPLIED -- ... -- > More importantly is %attrs;, which includes %coreattrs;: <!ENTITY % coreattrs "id ID #IMPLIED -- ... -- > Thus, an <input/> has *both* //input/@id AND //input/@name attributes. //input/@id is the ID for that *specific* <input/> control (to be used with scripting), while //input/@name is used for "grouping" the radio buttons (all radio buttons part of the same group require the same value for //input/@name). Thus we have a 1:1 mapping between our proposal and XHTML: //form:radio/@form:name :: //input/@id //form:radio/@form:group-name :: //input/@name If they want to follow W3C standards... We're following the W3C standards. Maybe. Now that I take a closer look at the ODF XML we're generating, I'm seeing a //form:radio/@form:id attribute. What does this attribute contain, and (more importantly) how is it set? Remember the original problem: Excel uses Name for VBA purposes, and GroupName for grouping purposes. Whatever we do needs to provide the same support. If we could use //form:radio/@form:name for the group name support and //form:radio/@form:id for scripting control name support (as a ~direct analog to what HTML does), this would support our requirements as well. The problem I see is in setting the //form:radio/@form:id attribute -- I don't see how/where it's set. At least, I don't see anything "obvious" in the Control Properties dialog. Perhaps all we need to do is expose this attribute in the Control Properties dialog? Spoke with Florian, and he *thinks* that the //form:radio/@form:id attribute is used for metadata purposes, so repurposing it for scripting purposes would likely be Bad. So I stand by the first half of my previous comment: we have a 1:1 mapping between our proposal and XHTML: //form:radio/@form:name :: //input/@id //form:radio/@form:group-name :: //input/@name So if the TC's issue is that we're not following W3C standards, I strongly disagree with their assessment. Jonathan, I didn't have time, yet, to think deeper about this, so everything I say is ... preliminary. Nonetheless, some quick notes. The id as found in the file format is not used at runtime, it's just for knitting the control models with the shapes which the control models belong to. Upon saving, they're generated on the fly, upon loading, they're discarded when loading is finished. I also thought about re-defining the name as some kind of ID, but unfortunately, I think this would break existing documents / ODF processors. I see the analogy to XHTML you pointed out, but my gut feeling (let me repeat: I didn't examine this path in detail) is we would have needed to care for this from the beginning, and now it's too late to give the existing form:name attribute the semantics of an ID. For the inconsistency as such ... (note that what I wrote here is from what I heard from people who attended the meetings - which means I could be arbitrarily far from the truth, even if I don't think so. Florian should have a better understanding of the current state of the proposal). I don't think the TC said that the duality of form:name and form:group-name is inconsistent, but the fact to use an attribute other than "name" for group definition. I agree to your description of how the name/group-name pair could equal the id/name pair in XHTML, but as said, I think re-defining the name attribute this way would be incompatible with the existing ODF spec/processors. Also, can we perhaps do it the other way round (again: didn't think this to an end, just a rough idea)? I.e., what if we export the GroupName property as form:name (for ratio buttons only, perhaps), and put the Name property into a new (to-be-created) attribute? Would this be feasible? Would such an other attribute be easier to define? Finally, I don't think it would be a good idea /me overtaking the proposal, but what I plan to do is to speak up in the comments list of the TC, ask about the status of the proposal, and describe why I think such an attribute would be useful. I am absolutely aware of the fact that this would be from the perspective of a developer of an Office Productivity Suite, and we might have a topic here where this perspective, and the perspective of the upholders of the file format, are ... incompatible. But perhaps it's worth something. I'm not sure I can do it this week, and I'm on a conference next week, so it might have to wait a few days more. And really finally, I would appreciate if we'd move this discussion to a mailing list, this would be more convenient :) I'm adding this comment to all open issues with Issue Type == PATCH. We have 220 such issues, many of them quite old. I apologize for that. We need your help in prioritizing which patches should be integrated into our next release, Apache OpenOffice 4.0. If you have submitted a patch and think it is applicable for AOO 4.0, please respond with a comment to let us know. On the other hand, if the patch is no longer relevant, please let us know that as well. If you have any general questions or want to discuss this further, please send a note to our dev mailing list: dev@openoffice.apache.org Thanks! -Rob I rechecked this in Apache OpenOffice 4.0.0 (and AOO 3.4.0) on Windows 7 Professional (32 bits).
With the first test file (test.sxc from the first attachment) the issue can not be reproduced.
With the complex use case ("Bleistahl -PSA 15-03-06.xls" from the second attachment), the problem still exists. In MS Office 2010, the highlighted percentages on the Summary sheet are:
42, 34, 23, 26 and 39.
In AOO 3.4.0, AOO 4.0.0 (and LibreOffice 4.0.4.2 and 4.1.0.4), the percentages are:
42, 34, 23, 26 and 44.
These are also the values you see after exporting to ODS. (MS Office 2010 prompts to "repair" the imported ODS and then also displays these values.)
When you save the original XLS file as XLS 97/2000/XP with a different name, the highlighted percentages in AOO (and LibreOffice) are the same as before. The values in MS Office 2010, however, are now:
42, 28, 22, 24, 40.
For the differences in the fifth value, see for example, the grouping issues on the Price sheet, cells E10, E12 and E13-E15.
|