User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1 See 1.0 2nd ed test suite test case 7.8.5.b. According to section 7.8.5 "If the specified argument does not identify a repeat, the function returns NaN." Reproducible: Always

The rub about this is whether we can get it into transformiix on the branches. I'm looking into if it is possible to return NaN from nsXFormsUtilityService::GetRepeatIndex, but since NaN is a double in transformiix, I don't think I can without a change to transformiix anyhow.

talked to Jonas. Getting a change into transformiix in 1.8.0 will be difficult. I think that we can work around this by returning an error from GetRepeatIndex that will propogate back through xpath to us (in nsXFormsUtils::EvaluateXPath, I think). We can check the return code and if it is NS_XFORMS_GENERATE_NAN or something like that, then we'll have to build an xpath result that is the same as NaN (or force an xpath evaluation that we know will result in a NaN and return that.

original testcase plus test for index using non-repeat node's ID.

Here is the most correct approach...generating the NaN xpathresult in the xforms xpath source code. This is the patch for the trunk. The fixes for the branches will be exactly the same but in a different file since the directory structure for xpath is different between trunk and the 1.8 and 1.8.0 branches. If this fix (branches version) isn't accepted for both branches, then we should probably scrap this approach altogether and go with the rv approach where we'll propogate a error back through xpath to xforms and then have xforms generate the NaN. To keep the xforms code common between all branches and the trunk.

patch for the 1.8.x branches. Same code as the trunk patch. Just in the correct files for 1.8.x branches.

Just to be clear, while this change is in the xpath code, the changed code can only be called by the xforms xpath evaluator and thus only used by the xforms extension.

Comment on attachment 218560 [details] [diff] [review] patch for trunk I don't like the idea of turning all errors into NaN. For example if instantiating the xformsService fails you probably want to propagate that error. One solution might be to make GetRepeatIndex return a |PRInt32| instead and make -1 indicate that NaN should be returned. Of course, you could also simply make it return a double.

(In reply to comment #8) > (From update of attachment 218560 [details] [diff] [review] [edit]) > I don't like the idea of turning all errors into NaN. > > For example if instantiating the xformsService fails you probably want to > propagate that error. > > One solution might be to make GetRepeatIndex return a |PRInt32| instead and > make -1 indicate that NaN should be returned. > > Of course, you could also simply make it return a double. > If instantiating the xformsService fails, we do propogate that error. We just won't propogate any error from GetRepeatIndex. But that is fine. I can change the signature of GetRepeatIndex to PRUInt32 and return a -1 to trigger the NaN.

patch with the -1 return talked about earlier

Comment on attachment 220088 [details] [diff] [review] trunk patch with -1 return >+nsXFormsUtilityService::GetRepeatIndex(nsIDOMNode *aRepeat, PRInt32 *aIndex) ... >- /// >- /// @bug This should somehow end up in a NaN per the XForms 1.0 Errata (XXX) >- return repeatEle ? repeatEle->GetIndex(aIndex) : NS_OK; >+ return rv; Change this to |return NS_OK;| With that, r=me

updated patch with jonas' comment fixed.

Comment on attachment 220135 [details] [diff] [review] trunk patch with jonas fix If we get peterv's signoff for the trunk change, then I'll port the same patch for 1_8_BRANCH and 1_8_0_BRANCH, attach it, and ask approvals.

(In reply to comment #13) > (From update of attachment 220135 [details] [diff] [review] [edit]) > If we get peterv's signoff for the trunk change, then I'll port the same patch > for 1_8_BRANCH and 1_8_0_BRANCH, attach it, and ask approvals. I just checked it in on trunk.

Comment on attachment 220135 [details] [diff] [review] trunk patch with jonas fix Requesting approval1.8.0.5. Although this patch includes an interface change, it is only used by XForms (and we live in our own little parallel world :-) ).

Comment on attachment 220135 [details] [diff] [review] trunk patch with jonas fix approved for 1.8.0 branch, a=dveditz for drivers

checked into 1.8.0 for aaron

checked into 1.8.1