Issue 122840 - Ranged names don't work anymore
Summary: Ranged names don't work anymore
Status: CLOSED FIXED
Alias: None
Product: Calc
Classification: Application
Component: programming (show other issues)
Version: 4.0.0
Hardware: All All
: P3 Normal with 1 vote (vote)
Target Milestone: 4.0.1
Assignee: Oliver-Rainer Wittmann
QA Contact: Prachi
URL:
Keywords: regression
: 122886 (view as issue list)
Depends on:
Blocks:
 
Reported: 2013-07-25 15:41 UTC by franky.chambers
Modified: 2017-05-20 10:34 UTC (History)
10 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---
jsc: 4.0.1_release_blocker+


Attachments
Spreadsheet to compute different parameters of an electric vehicle. (439.12 KB, application/vnd.oasis.opendocument.spreadsheet)
2013-07-25 15:41 UTC, franky.chambers
no flags Details
2.4.3 vs. 4.0.0 screenshot (604.23 KB, image/jpeg)
2013-07-25 19:07 UTC, Edwin Sharp
no flags Details
Zip file containing many examples. (447.28 KB, application/octet-stream)
2013-07-29 17:11 UTC, don1225
no flags Details
Fix patch (898 bytes, patch)
2013-09-02 02:20 UTC, Clarence GUO
orw: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description franky.chambers 2013-07-25 15:41:39 UTC
Created attachment 81143 [details]
Spreadsheet to compute different parameters of an electric vehicle.

Names in my spreadsheet don't work anymore with OOO 4.0.
See attachement.
Comment 1 Edwin Sharp 2013-07-25 16:34:31 UTC
Attachment displays OK in OOo 2.4.3.
Attachment shows #NAME? and no graph with Rev. 1503704 Win 7.
Comment 2 Oliver Brinzing 2013-07-25 17:37:15 UTC
confirming, looks good in aoo3.4.1 but show #NAME? in aoo 4.0
Comment 3 Ariel Constenla-Haile 2013-07-25 17:39:23 UTC
Ccing Regina
@Regine: you seem to have older revisions, can check this as you did with Bug 122822? Thanks!
Comment 4 Edwin Sharp 2013-07-25 19:07:25 UTC
Created attachment 81146 [details]
2.4.3 vs. 4.0.0 screenshot
Comment 5 Ariel Constenla-Haile 2013-07-25 19:10:39 UTC
(In reply to Edwin Sharp from comment #4)
> Created attachment 81146 [details]
> 2.4.3 vs. 4.0.0 screenshot

this is a regression from 3.4.1 to 4.0.0
Comment 6 Regina Henschel 2013-07-25 19:26:01 UTC
@Ariel: It is OK in r1378069, it is broken in r1381583.
Comment 7 Ariel Constenla-Haile 2013-07-25 19:50:43 UTC
(In reply to Regina Henschel from comment #6)
> @Ariel: It is OK in r1378069, it is broken in r1381583.

Then might be introduced by revision 1379349 - bug 120478 "Enhancement request: support sheet scoped named ranges in Calc"
Comment 8 Ariel Constenla-Haile 2013-07-29 16:33:18 UTC
*** Issue 122886 has been marked as a duplicate of this issue. ***
Comment 9 don1225 2013-07-29 17:11:14 UTC
Created attachment 81197 [details]
Zip file containing many examples.
Comment 10 Oliver-Rainer Wittmann 2013-08-28 05:52:05 UTC
I am having a deeper look.
Comment 11 Oliver-Rainer Wittmann 2013-08-29 12:09:47 UTC
(In reply to Oliver-Rainer Wittmann from comment #10)
> I am having a deeper look.

I had no success in my defect cause hunting.
Thus, please somebody else step in
Comment 12 Regina Henschel 2013-08-29 22:28:13 UTC
I was not able to produce such error with a new spreadsheet. The named expressions with ADDRESS, INDIRECT, and VLOOKUP work in my spreadsheet.

The attached file is far too large and complex. It cannot be analyzed in a reasonable time. Please provide a much smaller file, that shows the error.
Comment 13 Joe Smith 2013-08-30 00:24:45 UTC
> The attached file is far too large and complex. It cannot be analyzed
> in a reasonable time. Please provide a much smaller file, that shows the error

I provided a small sample document and steps for reproducing the problem under bug 122886
Comment 14 Clarence GUO 2013-08-30 08:44:25 UTC
Hi~ I find a problem in 120478 which cause this defect.
In 120478, zhaoshzh added support for scope of name range. MS Excel can define a same name to a range on different sheets. For example defines "MyRange" to "Sheet1.A1:A4" with scope in sheet1 and defines "MyRange" again to "Sheet2.A1:A4" with scope in sheet2.
In order to add this support, he changed code in ScXMLImport::SetNamedRanges(),
from
xNamedRanges->addNewByName((*aItr)->sName, sTempContent, aCellAddress, GetRangeType((*aItr)->sRangeType));
to
xNamedRanges->addNewByScopeName( sTabName, (*aItr)->sName, (*aItr)->sContent, aCellAddress, GetRangeType((*aItr)->sRangeType) );
addNewByScopeName is a new method, the difference with addNewByName is it added a new parameter sTabName in order to distinguish the scope. Others are same. But note he also changed the third parameter of the new method. In the old method, that parameter is sTempContent, it is string "0". However, in the new method he passed (*aItr)->sContent into, (*aItr)->sContent is the real content of the name. For example, for above illustration, "Sheet1.A1:A4" is the content of range name "MyRange".
I'm not sure whether it is his pen mistake, but from logic, as he only add a new parameter for scope identification, the old parameters should not be changed. So I just change (*aItr)->sContent back to sTempContent, then it works.
And there's no problem on 120478 sample file with the fix. I did some test, haven't find problem.
But I don't know why we need to pass a "0" string as content into the method. Does anybody knows that?
Comment 15 Oliver-Rainer Wittmann 2013-08-30 14:37:38 UTC
(In reply to Clarence GUO from comment #14)
> 
> [snip]
> 
> I'm not sure whether it is his pen mistake, but from logic, as he only add a
> new parameter for scope identification, the old parameters should not be
> changed. So I just change (*aItr)->sContent back to sTempContent, then it
> works.
> And there's no problem on 120478 sample file with the fix. I did some test,
> haven't find problem.
> But I don't know why we need to pass a "0" string as content into the
> method. Does anybody knows that?

That is the defect cause and the solution as well.
Applying "0" as content when adding a named range is done by purpose as far as I can see from the code in <ScXMLImport::SetNamedRanges()>. There two loops in this methods. The first inserts the named ranges. The second applies the content incl. the grammar to the inserted ranges. Thus, I conclude that it is done by purpose.

I also agree to your conclusion that by bug 120478 only the <scope name> has been added for the insertion of the named ranges.

Please contribute the corresponding patch
Comment 16 Oliver-Rainer Wittmann 2013-08-30 14:41:17 UTC
fix available
Comment 17 Clarence GUO 2013-09-02 02:20:54 UTC
Created attachment 81430 [details]
Fix patch

Root cause and fix analysis please reference my previous comment
Comment 18 Oliver-Rainer Wittmann 2013-09-02 08:12:04 UTC
Comment on attachment 81430 [details]
Fix patch

patch looks good.
Comment 19 Oliver-Rainer Wittmann 2013-09-02 08:16:57 UTC
taking over to integrate the fix into trunk and into branch AOO401 (if release blocker flag is granted.
Comment 20 SVN Robot 2013-09-02 09:17:45 UTC
"orw" committed SVN revision 1519366 into trunk:
122840: <ScXMLImport::SetNamedRanges()> - adding named ranges with temporary ...
Comment 21 jsc 2013-09-02 11:19:13 UTC
approve showstopper request
Comment 22 Oliver-Rainer Wittmann 2013-09-02 12:10:18 UTC
fixed on trunk and branch AOO401 (rev. 1519400)
Comment 23 SVN Robot 2013-09-02 12:10:47 UTC
"orw" committed SVN revision 1519400 into branches/AOO401:
122840: <ScXMLImport::SetNamedRanges()> - adding named ranges with temporary ...
Comment 24 fanyuzhen 2013-09-05 02:46:32 UTC
The latest available build in cwiki is AOO401m2(Build:9711)  -  Rev. 1518667, waiting for the build AOO401 (rev. 1519400)
Comment 25 Prachi 2013-09-06 01:59:56 UTC
This defect is not fixed in 4.0.1 Rev. 1518667 . 
OS -Win 7.
Spreadsheet still shows #NAME? and no graph with Rev. 1518667
Comment 26 Ariel Constenla-Haile 2013-09-06 02:06:07 UTC
(In reply to Prachi from comment #25)
> This defect is not fixed in 4.0.1 Rev. 1518667 . 
> OS -Win 7.
> Spreadsheet still shows #NAME? and no graph with Rev. 1518667

Per comment 23 this should be fixed with revision 1519400
1518667 < 1519400
Comment 27 Ariel Constenla-Haile 2013-09-06 02:28:18 UTC
back to fixed