528166 - mozIStorageStatement getParameterIndex causes NS_ERROR_ILLEGAL_VALUE

Status: RESOLVED INVALID

(Firefox :: Bookmarks & History, defect)

Description

[Andreas Wagner [:TheOne] [use NI]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)

My script is

calling statement.getParameterIndex(":myparam")

which works in Firefox 3.5.*
but that broke somewhen in 3.6 with:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [mozIStorageStatement.getParameterIndex]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: chrome://webreview/content/chronicle.js :: loadTreeDomains :: line 287"  data: no]

I tested various 3.6 nightlies and traced it back to May 2009-

2009-05-12 build works
2009-05-13 build breaks

If I read http://hg.mozilla.org/mozilla-central/log/f18d934ac5eb correctly, only rev 1391-1400 have been checked in between these two nightlies, but none of them changes mozStorageStatement.cpp where getParameterIndex is in.

Hope it helps anyway to find the bug.

Reproducible: Always

Comment 1

[John P Baker]

You seem to have the wrong year for your revision log.

This would seem to be bug 490833 which AFAICT removed the ':' from the parameter name.

> -   * @param aName The name of the parameter you want the index for.
> -   * @return The index of the named parameter.
> +   * @param aName
> +   *        The name of the parameter you want the index for.  This does not
> +   *        include the leading ':'.
> +   * @returns the index of the named parameter.
>     */
>    unsigned long getParameterIndex(in AUTF8String aName);

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Can you please give us the changeset ids for those builds? You get those when opening about:buildconfig. Thanks.

When looking at the above dates I wonder about your first statement on IRC that 3.6b1 is working fine while current nightlies are broken. Further do you have a simple testcase you can attach to the bug?

(In reply to comment #1)
> This would seem to be bug 490833 which AFAICT removed the ':' from the
> parameter name.

No this bug has added the feature that we only allow names with a : in-front.

Comment 3

[Marco Bonardo [:mak]]

> (In reply to comment #1)
> > This would seem to be bug 490833 which AFAICT removed the ':' from the
> > parameter name.
> 
> No this bug has added the feature that we only allow names with a : in-front.

this is pretty clear
> +   *        The name of the parameter you want the index for.  This does not
> +   *        include the leading ':'.

DOES NOT include the leading :

so getParameterIndex(":myparam") is clearly wrong and should be getParameterIndex("myparam") after bug 490833

Comment 4

[Marco Bonardo [:mak]]

the title of the bug could look wrong, but actually before that change you could call the param #param or @param or -param, now you can only use :param since the ":" is already included by the API. so the title is correct.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, so changing to getParameterIndex("myparam") should do the trick. Looks like this is invalid then?

Comment 6

[Andreas Wagner [:TheOne] [use NI]]

> You seem to have the wrong year for your revision log.

Damn, right, I was going (a bit) to far back.

> Can you please give us the changeset ids for those builds? You get those when
> opening about:buildconfig. Thanks.

2009-05-12: http://hg.mozilla.org/mozilla-central/rev/ed38105c9c2a
2009-05-13: http://hg.mozilla.org/mozilla-central/rev/da613c9fae8c

> When looking at the above dates I wonder about your first statement on IRC that
> 3.6b1 is working fine while current nightlies are broken.

I can explain this myself. I tested my add-on against 3.6b1 (at least I thought so) when it came out and didn't get an error. Today I wondered how far I have to go back until the error goes away, I tested again with 3.6b1 and voila the bug happens there also. So I must have used accidentaly the wrong version (probably a 3.5.x) the first time. 

> Further do you have a simple testcase you can attach to the bug?

> so getParameterIndex(":myparam") is clearly wrong and should be
> getParameterIndex("myparam") after bug 490833

Do you still need a testcase or is this really bug 490833? Also, how shall I maintain my extension for 3.0-3.6 if one version needs the ":" and another one does not even allow it?

Comment 7

[Andreas Wagner [:TheOne] [use NI]]

> I can explain this myself.

I can NOT explain this myself...

Comment 8

[Marco Bonardo [:mak]]

(In reply to comment #6)
> Do you still need a testcase or is this really bug 490833? Also, how shall I
> maintain my extension for 3.0-3.6 if one version needs the ":" and another one
> does not even allow it?

unless you want to do version-checking in the extension, i suppose you could have to provide 2 different add-ons, one compatible till 3.5.x and one from 3.6.*

Comment 9

[Marco Bonardo [:mak]]

but ideally you could be able to check version and add the ":" where appropriate.

Comment 10

[Andreas Wagner [:TheOne] [use NI]]

That will force me to make my add-on even more complicated and bloated. 
I feel sorry for the one of my AMO editor colleagues that will review this add-on (whenever that happens). I know how they will feel.
That's why more complicated add-ons likely take longer to be reviewed.

Ok, mark the bug as invalid and mark me as frustrated.

Comment 11

[Marco Bonardo [:mak]]

well, you just need an helper like getParamName("yourparam") that will return the correct string, should be a few lines of code.

Comment 12

[Shawn Wilsher :sdwilsh]

You can probably simplify your code so you don't have to work around this bug at all.  What is your use case for this API?  I'm willing to help you get a solution that works in 3.5 ad 3.6.

Comment 13

[Shawn Wilsher :sdwilsh]

Also, 3.0 will be end-of-lifed at the end of December/January, so you don't have to support it much longer anyway.

Comment 14

[Andreas Wagner [:TheOne] [use NI]]

(In reply to comment #12)
> You can probably simplify your code so you don't have to work around this bug
> at all.  What is your use case for this API?  I'm willing to help you get a
> solution that works in 3.5 ad 3.6.

Thank you for your offer.

Yeah, 3.0 is the main reason I need getParameterIndex cause I can't use the named parameter for that version. I'm using my own database (together with Places) so I can't rely on the Places API provided by Firefox.

I already changed the code and re-uploaded it to AMO.

I would be gladful if you can tell me what's the best method to find out the current version of Fx.

Currently I'm using:

function isFirefox36() {
	var version = Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsIXULAppInfo).version;
	if (parseFloat(version.substring(0,3)) >= "3.6") {
		return true;
	} else {
		return false;
	}
}

which is a bit ugly I think.

If this is too much offtopic for this bug, we can talk in IRC if you want.

Comment 15

[Shawn Wilsher :sdwilsh]

You can use the named parameters on 3.0 as well, you just have to manually wrap the statement.  See https://developer.mozilla.org/en/mozIStorageStatementWrapper for more details.  This works through 3.6.

Comment 16

[Andreas Wagner [:TheOne] [use NI]]

is this going to be added to

https://developer.mozilla.org/en/Updating_extensions_for_Firefox_3.6 or
https://developer.mozilla.org/en/Firefox_3.6_for_developers ?

I wanted to do this, but have been told not to do it (don't know anymore who told me).

Comment 17

[Eric Shepherd [:sheppy]]

I'm confused about what exactly needs to be done to the docs here.

Comment 18

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv

Comment 20

[:Gavin Sharp [email: [e-mail]]]

Sheppy: removal of getParameterIndex() and the addition of something along the lines of bug 531462 comment 2 and on https://developer.mozilla.org/en/mozIStorageStatement , I'm thinking.