Bug Hunting Session
Bug 51309 - Names of DatabaseRanges should not be case sensitive
Summary: Names of DatabaseRanges should not be case sensitive
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
3.6.0.0.beta1
Hardware: Other All
: medium normal
Assignee: call.protected
URL:
Whiteboard: target:4.0.0
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on:
Blocks:
 
Reported: 2012-06-21 11:04 UTC by Andreas Säger
Modified: 2015-12-15 16:47 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
DIFF of modifications to call findByUpperName instead of findByName (15.40 KB, patch)
2012-10-26 13:47 UTC, call.protected
Details
new patch with modifications. (14.19 KB, patch)
2012-10-26 14:52 UTC, call.protected
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Säger 2012-06-21 11:04:28 UTC
Some of my macros break because of case-sensitive DBRange names.
nrg = ThisComponent.NamedRanges.getByName("sal") gets named range "SAL"
addrg = ThisCOmponent.Sheets(0).getCellRangeByName("a1") gets cell A1
dbrg = ThisComponent.DatabaseRanges.getByName("apo_umsatz") raises no such element exception instead of returning database range "APO_Umsatz"

Like anything else, DBRange names are case-insensitive in the UI.
Comment 1 Alex Thurgood 2012-07-02 00:51:46 UTC
Andreas,

Can you provide us with a test case please ?

This might be Kohei's or Markus' department, they worked on the DBrange stuff.

Thanks,

Alex
Comment 2 Markus Mohrhard 2012-07-10 04:18:57 UTC
We don't need a test document for this one. I just need to think a bit about it if and then how to change it.
Comment 3 Markus Mohrhard 2012-07-10 04:23:55 UTC
If anyone wants to take this one it is a trivial change in dbdata.cxx for ScDBData::less::operator() and to make sure that we use and set everywhere the upper name.

Let me make it an EasyHack!
Comment 4 Daniel 2012-08-04 02:54:44 UTC
I'll try it out, but this is the first bug I'll be taking on, so I'm not sure if I'll understand what to do.
Comment 5 Markus Mohrhard 2012-08-04 08:48:39 UTC
Hey,

the database range is internally represented by ScDBData (http://opengrok.libreoffice.org/xref/core/sc/inc/dbdata.hxx#47). The idea is now to replace all calls to ScDBCollection::NamedDBs::findByName with calls to ScDBCollection::NamedDBs::findByUpperName and then remove the method findByName.

The easiest way should be to remove the method and then recompile and fix all compiler errors. If you have a rtl::OUstring that is not in uppercase you can transform it with ScGlobal::pCharClass->uppercase(aName) which returns the uppercase version of aName.
Comment 6 Markus Mohrhard 2012-08-04 08:49:40 UTC
If you have any problems please ask here or on the dev mailing list.
Comment 7 Markus Mohrhard 2012-10-17 10:11:24 UTC
I'm putting this easy hack back into NEW. There seems to be no progress from the assignee.
Comment 8 call.protected 2012-10-25 15:38:29 UTC
I'll try to take care of that in a short time (one-two days). I'll notify you anyway.
Comment 9 Markus Mohrhard 2012-10-25 21:32:19 UTC
(In reply to comment #8)
> I'll try to take care of that in a short time (one-two days). I'll notify
> you anyway.

Great. Please ask here, on IRC in #libreoffice-dev or at the developer mailing list if you need any help.
Comment 10 call.protected 2012-10-26 03:39:39 UTC
Thank you. 

I just need an advice : don't you think it would be nicer to call :  ScGlobal::pCharClass->uppercase(aName)" inside the method : "ScDBCollection::NamedDBs::findByUpperName(const ::rtl::OUString& rName)" on the argument "rName" passed ?

As we will always compare upper names in the findByUpperName methods, it will be possible to call the function with an case-insensitive rtl::OUString&. 

For example, the call : 

ScDBData* pDBData = Doc->GetDBCollection()->getNamedDBs().findByName("DBRange1");

will then be replaced by :

ScDBData* pDBData = Doc->GetDBCollection()->getNamedDBs().findByUpperName("DBRange1");

instead of doing the second modification or a call to uppercase(aName) for the string "DBRange1" to be "BBRANGE1".
Comment 11 Markus Mohrhard 2012-10-26 04:31:38 UTC
This would give some performance problems. We need to check now or later all places where we call uppercase now if we really need to do it. Half of them can live without ot.

Especially in large documents we want only a few uppercase calls so in future we will save the uppercase version where possible and only have the few places that really need a non uppercase version have to explicitly transform it to uppercase.

Leaving the non uppercase method might give a false impression that there is no penalty for calling the non uppercase version.
Comment 12 call.protected 2012-10-26 13:47:33 UTC
Created attachment 69112 [details]
DIFF of modifications to call findByUpperName instead of findByName

- ScDBData::less::operator() is modified to compare upper strings

- All the calls to findByName() are now findByUpperName().

- In case of a call to findByUpperName by an argument, the function ScGlobal::pCharClass->uppercase(aName) was used to ensure upper case.

- The definitions of FindByName() are deleted.

Should solve the problem.
Comment 13 Markus Mohrhard 2012-10-26 14:02:52 UTC
(In reply to comment #12)
> Created attachment 69112 [details] [review]
> DIFF of modifications to call findByUpperName instead of findByName
> 
> - ScDBData::less::operator() is modified to compare upper strings
> 
> - All the calls to findByName() are now findByUpperName().
> 
> - In case of a call to findByUpperName by an argument, the function
> ScGlobal::pCharClass->uppercase(aName) was used to ensure upper case.
> 
> - The definitions of FindByName() are deleted.
> 
> Should solve the problem.

There are some small problems in the patch. 

GetFuncCollection()->findByName should not be changed.

In ScAreaLink::FindExtRange you changed one findByUpperName to findByName.

Can you send the patch with a license statement to the dev mailing list?
Comment 14 call.protected 2012-10-26 14:52:37 UTC
Created attachment 69115 [details]
new patch with modifications.

I have undo these modifications to solve the small problems.

I post the patch there, I'll send it to the mailing list too.

Thanks !
Comment 15 Not Assigned 2012-11-19 13:15:12 UTC
Mathieu D committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=aafe2fcbcb754b2fff8d7c3bf1f61781df259b67

database names are case insensitive, fdo#51309



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 16 Markus Mohrhard 2012-11-19 13:18:34 UTC
Thanks a lot for your great patch. Will be available in 4.0
Comment 17 Robinson Tryon (qubit) 2015-12-15 16:47:17 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp )
[NinjaEdit]