Closed Bug 528166 Opened 15 years ago Closed 15 years ago

mozIStorageStatement getParameterIndex causes NS_ERROR_ILLEGAL_VALUE

Categories

(Firefox :: Bookmarks & History, defect)

Product:
Firefox
Component:
Bookmarks & History
Version:
3.6 Branch
Platform:
x86
Windows NT
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: TheOne, Unassigned)

References

Details

(Keywords: dev-doc-needed) 

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
Priority: -- → P2
Version: unspecified → 3.6 Branch
Summary: statement getParameterIndex causes NS_ERROR_ILLEGAL_VALUE → mozIStorageStatement getParameterIndex causes NS_ERROR_ILLEGAL_VALUE
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);
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.
Keywords: regression, regressionwindow-wanted
Priority: P2 → --
> (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
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.
Ok, so changing to getParameterIndex("myparam") should do the trick. Looks like this is invalid then?
> 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?
Priority: -- → P2
> I can explain this myself.

I can NOT explain this myself...
(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.*
but ideally you could be able to check version and add the ":" where appropriate.
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.
Priority: P2 → --
well, you just need an helper like getParamName("yourparam") that will return the correct string, should be a few lines of code.
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
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.
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.
(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.
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.
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).
I'm confused about what exactly needs to be done to the docs here.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
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.
You need to log in before you can comment on or make changes to this bug.