Apache OpenOffice (AOO) Bugzilla – Issue 108876
[From Symphony]Application.Range does not work well
Last modified: 2017-05-20 11:29:24 UTC
1. Root Cause: Currently, we did not support Application.Range(Range1, Range2), Range1 or Range2 is not the range of current active sheet. We always consider these two ranges are from the current active sheet. And Application.Range(...) does not set the Parent property. 2. Resolution: Add the codes for supporting shortcut: Application.Range(Range1, Range2), Range1 or Range2 is not the range of current active sheet. If Range1 and Range2 are not in current active sheet, we do not use the active sheet, but use the sheet of Range1 and Range2. If Range1 and Range2 are not in the same sheet, we throw an exception. Return the Parent for Application.Range(...).
Created attachment 67556 [details] Fix for this issue
why is there a new m_xParent in ScVbaRange & ScVbaWorksheet? ( these are already available in the base class ), the existing parent is also already initialised in the constructor too :-/ ScVbaRange::ApplicationRange has the logic added to detect if Range1 && Range2 are ranges that are in different sheets then an exception is raised, isn't this logic already present in ScVbaRange::Range ? ( see the special application::Range logic when 'bForceUseInpuRangeTab' is set ) * the attached patch addes additional logic to test if the ranges are from a different document, but... isn't this test equally valid for ScVbaRange::Range( Range1, Range2 )? in other words should the test for whether Range1 & Range2 are from difference worksheets be moved to ScVbaRange::Range? what do you think?
>>why is there a new m_xParent in ScVbaRange & ScVbaWorksheet? ( these are already available in the base class ), the existing parent is also already initialised in the constructor too :-/ From Hui Li: Because m_xParent in the base class is weak reference, some time the parent of Range or Worksheet is created in C++ method, it is destructed when step out of the method, because m_xParent is weak reference. So Application.Range(...).Parent may be null. >>ScVbaRange::ApplicationRange has the logic added to detect if Range1 && Range2 are ranges that are in different sheets then an exception is raised, isn't this logic already present in ScVbaRange::Range ? ( see the special application::Range logic when 'bForceUseInpuRangeTab' is set ) * the attached patch addes additional logic to test if the ranges are from a different document, but... isn't this test equally valid for ScVbaRange::Range( Range1, Range2 )? in other words should the test for whether Range1 & Range2 are from difference worksheets be moved to ScVbaRange::Range? From Hui Li: Yes, can detect if Range1 and Range2 are ranges that are in different documents or sheets. Please see my new patch
Created attachment 69194 [details] New patch for this issue.
>Because m_xParent in the base class is weak reference, some time >the parent of Range or Worksheet is created in C++ method, it is destructed when >step out of the method, because m_xParent is weak reference. So >Application.Range(...).Parent may be null Right, this is a separate problem, I suggest you open a separate issue for it. Also. I'm sorry but I don't like the solution for getting the parent, 1. duplicating the parent member in the Range and Worksheet classes is very confusing. 2. many unnecessary worksheet objects will be created 3. this will impact performance badly we should either a) make mxParent in InheritedHelperInterfaceImpl be a hard reference or b) make sure the parent that is passed has a long life a) is imo not a good idea, I think it will possibly introduce many cyclic reference issues b) is a much better idea, we should probably access Workbook/Worksheet objects directly from basic ( where they already have the correct lifecycle ) - when issue #111144 has a proper solution then this should hopefully be even easier, in the mean time we can I think already access these from basic so, in summary I think the best thing to do is to remove the getRangeParent bits from this patch and create a new issue for that part. The remaining part of the patch ( to do with Application.Range( Range1, Range2 ) etc. looks good to me
Created attachment 69389 [details] Test document for this issue
Created attachment 69390 [details] New patch: Only fix the issue of Application.Range(...), Range.Parent issue does not fix in this patch
I post new patch(patch_alt_removeparent.diff), only fix the issue of Application.Range(...), do not fix the problem of Range.Parent. I created a new issue for the problem of Range.Parent: http://qa.openoffice.org/issues/show_bug.cgi?id=111505
Reset assigne to the default "issues@openoffice.apache.org".