Last Comment Bug 376371 - [1.1] Add support for resource element on submission to allow more flexibility
: [1.1] Add support for resource element on submission to allow more flexibility
Status: RESOLVED FIXED
: fixed1.8.0.12, fixed1.8.1.4
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: unspecified
: All All
: -- enhancement with 2 votes (vote)
: ---
Assigned To: Merle Sterling
: Stephen Pride
Mentors:
http://www.w3.org/TR/xforms11/#submit...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-03 10:01 PDT by Steve Speicher
Modified: 2016-07-15 14:46 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
test case (1.03 KB, application/xhtml+xml)
2007-04-12 09:34 PDT, Steve Speicher
no flags Details
testcase 2 (2.57 KB, application/xhtml+xml)
2007-05-10 15:55 PDT, aaronr
no flags Details
patch (8.85 KB, patch)
2007-05-10 18:56 PDT, Merle Sterling
aaronr: review+
Details | Diff | Splinter Review
patch2 (8.75 KB, patch)
2007-05-11 12:21 PDT, Merle Sterling
bugs: review+
Details | Diff | Splinter Review
patch3 (8.55 KB, patch)
2007-05-15 15:28 PDT, Merle Sterling
no flags Details | Diff | Splinter Review
patch with changes for 1.8 branch (2.53 KB, patch)
2007-05-16 17:16 PDT, Merle Sterling
doronr: review+
aaronr: review+
Details | Diff | Splinter Review

Description Steve Speicher 2007-04-03 10:01:44 PDT
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 2007-04-03 10:17:01 PDT
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 2007-04-12 09:34:54 PDT
Created attachment 261390 [details]
test case

Should be how it works in 1.1
Comment 3 aaronr 2007-05-10 15:55:02 PDT
Created attachment 264416 [details]
testcase 2

patch that fixes this bug should also pass testcase 2, I think.
Comment 4 Merle Sterling 2007-05-10 18:56:42 PDT
Created attachment 264433 [details] [diff] [review]
patch
Comment 5 aaronr 2007-05-11 11:53:53 PDT
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 2007-05-11 12:21:46 PDT
Created attachment 264508 [details] [diff] [review]
patch2
Comment 7 alexander :surkov 2007-05-11 21:04:33 PDT
I would prefer to use nsIContent and nsIAtom, string usage is not very protected from typos.
Comment 8 Olli Pettay [:smaug] 2007-05-12 02:17:26 PDT
nsIContent is sort-of-internal-interface for gecko, and IMO, we should
keep its usage at minimum.
Comment 9 Leigh L. Klotz, Jr. 2007-05-15 09:53:46 PDT
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] 2007-05-15 14:58:09 PDT
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 2007-05-15 15:28:39 PDT
Created attachment 264926 [details] [diff] [review]
patch3

OK, just one nsAutoString.
Comment 12 Merle Sterling 2007-05-16 17:16:54 PDT
Created attachment 265068 [details] [diff] [review]
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 2007-05-17 09:12:58 PDT
checked in 'patch with changes for 1.8 branch' to trunk
Comment 14 aaronr 2007-05-17 09:39:20 PDT
checked in 'patch3' and 'patch with changes for 1.8 branch' to the 1.8 and 1.8.0 branches
Comment 15 aaronr 2007-05-18 12:15:47 PDT
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 2007-05-18 14:53:21 PDT
'patch with changes for 1.8 branch' is now back on trunk.  Resolving bug as fixed again.

Note You need to log in before you can comment on or make changes to this bug.