Apache OpenOffice (AOO) Bugzilla – Issue 59082
autoshapes with macros not imported into sc
Last modified: 2013-08-07 15:14:03 UTC
Hi Daniel, Seems that in excel autoshapes can have macro's associated with them. Currently OOo doesn't import that for excel ( and the other apps I guess ). So here's a little patch I put together to rectify that. You might pass your beady eye over it. I'd like to get this upstream, any help you could give with that would be appreciated. So the patch, its pretty simple, a) modifies ScDrawObjData to store the macro ( sc/sc/inc/userdat.hxx ) b) adds an eventsupplier interface to ScShapeObj, the format of the properties returned from the supplier match those used in presentation for "interaction events", thus xmloff handles things properly on import/export so the bindings are persisted correctly. (sc/source/ui/unoobj/shapeuno.cxx sc/inc/shapeuno.hxx) c) moved maMacroName & ReadMacro from XclImpTbxControlObj to XclImpDrawObjBase so that shape object can access this. The only piece of missing functionality is the context menu and ui. But... no new ui needs to be created ( so no new strings, translations etc ) it should be possible to re-use the existing presentation dialog ( sd/ source/ui/dlg/tpaction.cxx ) of course that would a) need to be moved to somewhere common that could be accessed by sc & sd ( I have no clue about ui :-( ) b) need to be modified so that a mode could be passed to it so that the choices in the list box are "RunMacro, No Action" whatya think?
Created attachment 32201 [details] patch
Created attachment 32202 [details] xl doc with autoshapes with macros
added cc:
DR->FL: please have a look at this feature. It adds a macro link to all custom shapes (currently only possible for form control objects). This is needed for compatibility to Excel.
@Noel: please rework the Excel filter part. I don't like this "hacky" public member maMacroName ;-)
added dr ( cc ) Daniel, FL >@Noel: please rework the Excel filter part. I don't like this "hacky" public >member >maMacroName ;-) your right of course, thats a bit naff ( totally forgot to fix that up when cleaning up the patch ), so great, thanks for spotting that :-) I upload a new patch before end of today ( 14/12/05 ) or tomorrow latest & you can let me know if its ok for you or not :-)
Created attachment 32377 [details] moved maMacroName to be private & added GetMacroName accessor
Urgh, we ( at go-ooo.org ) found a subtle problem with this patch. The patch uses a ScDrawObjData to store the macro name in the shape, this is not ideal because a) the ScDrawObjData ctor leaves some unintialiased data e.g. member bValidStart is never initialised b) on import even if no macro is associated with the shape then the ScDrawObjData is created. c) in the particular instance where this was observed to be causing a problem ( for arrows ) the repositioning code depended on bValidStart been initialised correctly which of course resulted in somewhat random badness ( depending on the value of the unitialised variable ) So, I've reworked the patch ( not radically ) instead of using a ScDrawObjData shape object to store the binding, shapes with associated macro now use a new type ( ScMacroInfo ) to store the binding. This ensures that we don't affect existing code that may use/depend on the state of ScDrawObjData data.
Created attachment 34512 [details] new patch version
FL->DR: I support this change for OOo 2.0.3.
FL: I support the patch for OOo 2.0.3 too.
retargeted
I had occasion to revisit this issue again because I came across the same situation for import of hyperlinks. So when I was creating the patch for hyperlinks ( see issue 66550 ) I folded in those comments ( from a private mail ) you had previously. Also issue 66550 has a patch that depends on the latest version of the patch ( with those modifications folded in ) for this issue So, what's the story with this issue, is it currently being worked on? ( I guess I'm really talking about the ui part ) if not, I think I may be able to get some time to look into the ui bits ( but if thats already being taken care of then there is no point in me looking at it ) imo 66550 should be fixed at the same time ( due to overlap ), can I do anything here to help? ( with either issue )
Created attachment 37232 [details] patches with comments folded in
Created attachment 37343 [details] latest version of patch ( with ctx menu for assign/remove macro )
Created attachment 37344 [details] screenshot showing new macro assign context menu
Created attachment 37345 [details] screenshot showing new macro assign dialog
Created attachment 37346 [details] test document ( used for the screenshots )
Created attachment 37348 [details] the xls version of document
There was no time for me to look into the UI parts, so please take care of this. Also, we need a specification for this, should not be that difficult with the specification template. For now, I have to retarget this issue to 2.x, but we can still integrate it into 2.0.4, if the patch and spec are ready in time.
spec is available from here http://specs.openoffice.org/calc/compatibility/AutoShapeGraphicHyperlinkAndMacroSupport.odt the patches already contain the ui bits
npower->dr so, the spec is basic, please feel free to help, contribute, push, take ownership even ( I really don't care, I'm not possessive ) etc. from your side. I have put prelim i-team members in there etc. some names are missing, you could help fill those in also. If this is to have any chance of make 2.0.4 it will need a good shove, You could help if you can get agreement from re or whoever from your side to allow the resources to be done seperately ( to meet the ui freeze deadline) that would give us some space. But all of this hangs on getting the spec approved asap
specification has been updated by the User Experience team.
its great the spec, has been updated, but, I guess I'm a little fuzzy as to what happens next. Is the spec approved? (seems not) are the issues mentioned in the specification noted as future work? ( it's not clear to me ) or is this an initial review with more *stuff* to come? imo none of the comments in the specification should block the implementation of the feature, I'd like to comment on the 2 ( I could only see 2 anyway ) issues mentioned a) visual feedback that user has clicked the item, I completely agree with the comment. But, currently I have no clue how to do this, maybe someone in the framework team could help here. In anycase this is imo a minor cosmetic change that we could do later b) this should be something available across all apps. Sure, that's a valid point. Unfortunately this seems like it may be a little bit easier to say than to do. Currently each application seems to do their own filter bits and also offer specializations of the drawing objects ( why, I have no idea, presumably necessary for some valid reasons ) look at presentation for instance it seems to have even its own specialised custom interaction handling that is different again. So, although a good idea I would think this is something that should be handled in a broader rework effort. That effort however is outside the scope of this particular enhancement
target, started
patch (finally) integrated in SRC680/dr55 (OOo 2.3)
back to QA
Created attachment 46513 [details] TestCaseSpecification
verified in internal build cws_dr55
closed because fix available in src680_m225 (OOo 2.3)