instance() function not behaving as expected in official Test Suite

RESOLVED WONTFIX

Status

Core Graveyard
XForms
RESOLVED WONTFIX
10 years ago
a year ago

People

(Reporter: Koen Heene, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12

Hello,

The Mozilla XForms status page
(http://www.mozilla.org/projects/xforms/status-detail.html) states
that the function instance() is supported.

If had difficulties getting that to work. I went to the official
XForms test site and saw that the output is not as predicted by the test suite and also not as you would expect after having read the specs.

I currently work with FireFox 2.0.0.12 and the XForms extension 0.8.4.

Thank you for looking into this.

--
Koen

Reproducible: Always

Steps to Reproduce:
1. go to the URL above

Actual Results:  
The output of <xforms:output/> is not as predicted

Expected Results:  
That is described on the same test page.
(Reporter)

Comment 1

10 years ago
Created attachment 305213 [details]
an adapted version of the w3.org 7.11.1.a Test Page

The official w3.org test page has given the id="defaultInstance" to the instance they want to retrieve with instance() or instance(''). I don't find this possibility in the specs and therefore consider the Test Page erroneous.

The example in the attachment tries to correct that, but shows that the problems persists.
(Reporter)

Comment 2

10 years ago
Noteworthy in the attachment is the fact that the 2nd <xforms:output/> shows the label, but not the value. So it seems that instance() effectively retrieves the anonymous instance, but not the value identified by the XPath.

Comment 3

10 years ago
Thanks for the bug.  We do not correctly handle instance() with no parameter or empty string parameter.
(Reporter)

Comment 4

10 years ago
Thank you for replying.

Would it be useful to mention on the Mozilla XForms status page that instance() is only partly supported?

I suppose that the status page is a very important reference for developers.

Thank you,
Koen

Comment 5

10 years ago
updated the status page to include this bug
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

10 years ago
Blocks: 326372

Comment 6

10 years ago
Created attachment 305630 [details] [diff] [review]
patch for 1.8

This fix is for the 1.8 branch.  The trunk will have to do something similar, but it also needs to overcome the fact that xpath extension functions aren't allowed to have optional parameters, yet.
Attachment #305630 - Flags: review?(Olli.Pettay)

Updated

9 years ago
Attachment #305630 - Flags: review?(surkov.alexander)

Comment 7

9 years ago
Comment on attachment 305630 [details] [diff] [review]
patch for 1.8


>+          rv = document->GetElementById(instanceId, getter_AddRefs(instEle));
>+        }
>+      } else {
>+        instanceId = EmptyString();
>+        useDefaultInst = PR_TRUE;
>+      }
>  
>       PRBool foundInstance = PR_FALSE;
>       nsAutoString localname, namespaceURI;
>       if (instEle) {
>         instEle->GetLocalName(localname);
>         instEle->GetNamespaceURI(namespaceURI);
>         if (localname.EqualsLiteral("instance") && 
>             namespaceURI.EqualsLiteral(NS_NAMESPACE_XFORMS)) {
>           foundInstance = PR_TRUE;
>         }
>       }

this one and

>-      if (!foundInstance) {
>+
>+      // if we were given an instance id and couldn't find it, then bow out
>+      if (!foundInstance && !useDefaultInst) {
>         // We didn't find an instance element with the given id.  Return the
>         //   empty result set.
>         *aResult = resultSet;
>         NS_ADDREF(*aResult);


this one may be moved after rv = document->GetElementById(instanceId, getter_AddRefs(instEle)), right?

>+      if (!useDefaultInst) {
>+        instNode = do_QueryInterface(instEle);
>+        rv = xformsService->GetModelFromNode(instNode, 
>+                                             getter_AddRefs(modelInstance)); 
>+                                   
>+        NS_ENSURE_SUCCESS(rv, rv);
>+   
>+        rv = xformsService->IsNodeAssocWithModel(xfContextNode, 
>+                                                 modelInstance, 
>+                                                 &modelContainsNode);
>+        NS_ENSURE_SUCCESS(rv, rv);

why can't you have common algorithm here? It sounds "else" case is suitable for the "if" case?

>+      } else {
>+        // xforms stores the instance element that was used to create the
>+        // standalone instance document as a property of that document
>+        nsCOMPtr<nsIDOMDocument> instanceDOMDoc;
>+        xfContextNode->GetOwnerDocument(getter_AddRefs(instanceDOMDoc));

please check this, iirc document->GetOwnerDocument should return document.

with comments addressed r=me
Attachment #305630 - Flags: review?(surkov.alexander) → review+

Comment 8

9 years ago
> >+      } else {
> >+        // xforms stores the instance element that was used to create the
> >+        // standalone instance document as a property of that document
> >+        nsCOMPtr<nsIDOMDocument> instanceDOMDoc;
> >+        xfContextNode->GetOwnerDocument(getter_AddRefs(instanceDOMDoc));
> 
> please check this, iirc document->GetOwnerDocument should return document.
Nope. That returns null. http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#node-ownerDoc

(in gecko we have nsINode::GetOwnerDoc() which always returns the owner document)

Updated

9 years ago
Attachment #305630 - Flags: review?(Olli.Pettay) → review+

Comment 9

9 years ago
(In reply to comment #8)
> > >+      } else {
> > >+        // xforms stores the instance element that was used to create the
> > >+        // standalone instance document as a property of that document
> > >+        nsCOMPtr<nsIDOMDocument> instanceDOMDoc;
> > >+        xfContextNode->GetOwnerDocument(getter_AddRefs(instanceDOMDoc));
> > 
> > please check this, iirc document->GetOwnerDocument should return document.
> Nope. That returns null.
> http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#node-ownerDoc
> 
> (in gecko we have nsINode::GetOwnerDoc() which always returns the owner
> document)
> 

Thank you. Then I messed them :)
RIP xforms
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX
(Assignee)

Updated

a year ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.