376371 - [1.1] Add support for resource element on submission to allow more flexibility

Status: RESOLVED FIXED

(Core Graveyard :: XForms, enhancement)

Description

[Steve Speicher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070305 Minefield/3.0a3pre
Build Identifier: 

Need a mechanism to submit contents to a variable number of locations, which can all be configured to be different submission targets.  It would be ideal to specify the submission targets via instance data.  This is possible in XForms1.1 with the new <resource> element, that is a child of <submission>.

This mechanism will be used for the Integrating the Healthcare Enterprise (http://www.ihe.net) and it's Request Form for Data capture (RFD) profile.

Reproducible: Always

Comment 1

[aaronr]

I agree that this is worth doing sooner rather than later.  Good for RSS and Atom feed type interaction, too.

Please attach testcase.

Comment 2

[Steve Speicher]

Attachment: test case

Should be how it works in 1.1

Comment 3

[aaronr]

Attachment: testcase 2

patch that fixes this bug should also pass testcase 2, I think.

Comment 4

[Merle Sterling]

Attachment: patch

Comment 5

[aaronr]

Comment on attachment 264433 [details] [diff] [review]
patch

>Index: nsXFormsSubmissionElement.cpp
>===================================================================

>-  nsCOMPtr<nsIInputStream> stream;
>-  nsCAutoString uri, contentType;
>-  nsAutoString action;
>-  mElement->GetAttribute(NS_LITERAL_STRING("action"), action);
>-  CopyUTF16toUTF8(action, uri);
>+  nsCAutoString uri;
>+  GetSubmissionURI(uri);
> 
>+  nsCOMPtr<nsIInputStream> stream;
>+  nsCAutoString contentType;

nit: I'd leave things like they were where the declarations were grouped together since there is no way we can return before the call to SerializeData.  You just have to delete the declaration of action since we don't use it anymore.

> nsresult
>+nsXFormsSubmissionElement::GetSubmissionURI(nsACString &uri)
>+{
>+  // Precedence:
>+  // 1. If the submission element has a resource element as its first child,
>+  // the URI can be specifed by either the 'value' attribute or the string
>+  // content of the resource element. The resource element has precedence over
>+  // both the 'resource' and 'action' attributes.
>+  //
>+  // 2. If there is no resource element as the first child, the URI may be
>+  // specified by either the 'resource' or 'action' attributes with 'resource'
>+  // having precedence over 'action'.
>+  //
>+  // If no URI is specified via any of the above mechanisms we write a warning
>+  // message to the error console.
>+

Nice comments!!!

>+          rv = nsXFormsUtils::EvaluateNodeBinding(resourceElement, 0,
>+                                                  NS_LITERAL_STRING("value"),
>+                                                  NS_LITERAL_STRING("/"),
>+                                                  nsIDOMXPathResult::STRING_TYPE,
>+                                                  getter_AddRefs(model),
>+                                                  getter_AddRefs(xpRes),
>+                                                  &usesModelBind);

your 4th parameter is incorrect.  There shouldn't be a default ref on a xf:resource element.  Should probably be an empty string (EmptyString()).


>+  // If no URI is specified, write a warning to the console.
>+  if (value.IsEmpty() && resource.IsEmpty() && action.IsEmpty())

Why not just test if uri is empty?  By now it should have whatever the value is, right?


>Index: resources/locale/en-US/xforms.properties
>===================================================================

>+warnSubmitURI             = XForms Warning (9): Submission element must specify the URI via a resource element, resource attribute or action attribute

nit: Maybe change the warning to include that resource must be the first child element.  Like, "Submission element must specify the URI via a resource element (must be the first child element of submission), a resource attribute or an action attribute

with these fixed, r=me

Comment 6

[Merle Sterling]

Attachment: patch2

Comment 7

[alexander :surkov (:asurkov)]

I would prefer to use nsIContent and nsIAtom, string usage is not very protected from typos.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

nsIContent is sort-of-internal-interface for gecko, and IMO, we should
keep its usage at minimum.

Comment 9

[Leigh L. Klotz, Jr.]

It's a shame the WG didn't additionally approve a way of serializing XML leaf nodes as parameters into the resource with application/x-www-form-urlencoded serialization.  I foresee a lot of bugs with people doing <resource value="concat('a=', a, '&amp;', b, '&amp;')" /> where a and b contain characters that needs % coding (such as =).  There's no way for resource to know what interpretation is being given to its value, so there's no way for it to quote meta characters for you.  This note is here so that when people file bugs on resource/@value and concat not quoting meta characters they can see it's not Mozilla's fault.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 264508 [details] [diff] [review]
patch2


> nsresult
>+nsXFormsSubmissionElement::GetSubmissionURI(nsACString &uri)

Parameter should be nsACString& aURI

>+  nsAutoString resource, value, action;

Could there be just one nsAutoString
nsAutoString uri
which could be then just before error reporting code copied to aURI.
So wherever uri is used, use aURI, and wherever resource, value or action
is used, use uri. And have only one CopyUTF16toUTF8(uri, aURI) call.

With that change r=me

Comment 11

[Merle Sterling]

Attachment: patch3

OK, just one nsAutoString.

Comment 12

[Merle Sterling]

Attachment: patch with changes for 1.8 branch

To work on the 1.8 branch 'patch3' requires that we truncate the nsAutoString before assigning the xpath result from EvaluateNodeBinding because xpres->GetStringValue appends to the buffer rather than replacing it; also changing to always make the call to CopyUTF16toUTF8 to ensure that the returned uri from GetSubmissionURI is empty if no valid uri was found even if the caller passed in a non-empty string.


Apply 'patch with changes for 1.8 branch' to the trunk.
Apply 'patch3' and 'patch with changes for 1.8 branch' to the branch.

Comment 13

[aaronr]

checked in 'patch with changes for 1.8 branch' to trunk

Comment 14

[aaronr]

checked in 'patch3' and 'patch with changes for 1.8 branch' to the 1.8 and 1.8.0 branches

Comment 15

[aaronr]

I backed out 'patch with changes for 1.8 branch' on trunk to help mochitest debugging for ispiked.  I will put it back in when they are done.  I'm reopening bug so that we remember to do this.

Comment 16

[aaronr]

'patch with changes for 1.8 branch' is now back on trunk.  Resolving bug as fixed again.