310125 - failing to generate xforms-submit-error

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

In the spec, at: http://www.w3.org/TR/xforms/index-all.html#submit-event under
point #5, it says, "For an error response nothing in the document is replaced,
and submit processing concludes after dispatching xforms-submit-error".

However, we don't seem to be doing that.  If I submit to
http://www.xformstest.org/cgi-bin/echo.shakakahaka, for example, I get a 404
message back.  We display the error page, but I don't get the
xforms-submit-error to popup.  None of the other processors display the popup,
either.  

It seems to me that in ::OnStopRequest we should be checking to see if the
channel is a HttpChannel and if it is, then we should be checking
channel->GetResponseStatus to see if we got a 4xx or not.  Not just looking at
the value of status coming in.

Comment 1

[aaronr]

Attachment: testcase

testcase that shows the problem

Comment 2

[aaronr]

Attachment: less confusing testcase

Here is a less confusing testcase.  The other obsolete testcase WOULD show the
xforms-submit-error but only if you didn't have bugzilla in your trusted list
for cross domain submission.  This testcase should show the anticipated
failure.

Comment 3

[aaronr]

Attachment: another try at the testcase

Comment 4

[Doron Rosenberg (IBM)]

Attachment: le patch

Comment 5

[Doron Rosenberg (IBM)]

Submitting to a 404 sounds like a good error case.  Ask the WG? :)

Comment 6

[aaronr]

Comment on attachment 197582 [details] [diff] [review]
le patch

>Index: extensions/xforms/nsXFormsSubmissionElement.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsSubmissionElement.cpp,v
>retrieving revision 1.40
>diff -u -1 -0 -r1.40 nsXFormsSubmissionElement.cpp
>--- extensions/xforms/nsXFormsSubmissionElement.cpp	15 Sep 2005 11:37:37 -0000	1.40
>+++ extensions/xforms/nsXFormsSubmissionElement.cpp	27 Sep 2005 17:25:04 -0000
>@@ -408,20 +408,31 @@
>   LOG(("xforms submission complete [status=%x]\n", status));
> 
>   if (!mElement) {
>     return NS_OK;
>   }
> 
>   nsCOMPtr<nsIChannel> channel = do_QueryInterface(request);
>   NS_ASSERTION(channel, "request should be a channel");
> 
>   PRBool succeeded = NS_SUCCEEDED(status);
>+
>+  // Check if the return was a 4xx/5xx, since status will be set to true
>+  // if a 404 (for example) with a message body was returend.
>+  if (succeeded) {
>+    nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(channel);
>+
>+    if (httpChannel) {
>+      httpChannel->GetRequestSucceeded(&succeeded);
>+    }
>+  }
>+

I would say that GetRequestSucceeded isn't the right test, since it will return
false for anything that isn't a 2xx code.  We are supposed to special case "an
error response", not a 'non-success' response.	1xx and 3xx responses aren't
errors.  I would say that the test should be to check for 4xx and 5xx
responses.

Comment 7

[aaronr]

Allan, I told Kevin Kelly about this, but could you follow up on the next XForms
call?  We really need to know what the proper handling should be.  In the case
of a 4xx or 5xx http response, should we show any error page coming back from
the server AND send the xforms-submit-error?  Or just dispatch the
xforms-submit-error event?  Currently, Novell, fP and XSmiles all just show the
response from the server and do NOT dispatch the error.

Comment 8

[Allan Beaufour]

(In reply to comment #7)
> Allan, I told Kevin Kelly about this, but could you follow up on the next
> XForms
> call?  We really need to know what the proper handling should be.  In the case
> of a 4xx or 5xx http response, should we show any error page coming back from
> the server AND send the xforms-submit-error?  Or just dispatch the
> xforms-submit-error event?  Currently, Novell, fP and XSmiles all just show the
> response from the server and do NOT dispatch the error.

Re-reading this, and I must say that the spec. clearly says that on an "error response" nothing is done. By all means, a 404 is an error response. My interpretation is that, we should dispatch the error and do nothing else. I'll post to www-forms and ask though

Comment 9

[Allan Beaufour]

http://lists.w3.org/Archives/Public/www-forms/2006Apr/0009.html

Comment 10

[Allan Beaufour]

(In reply to comment #7)
> Allan, I told Kevin Kelly about this, but could you follow up on the next
> XForms
> call?  We really need to know what the proper handling should be.  In the case
> of a 4xx or 5xx http response, should we show any error page coming back from
> the server AND send the xforms-submit-error?  Or just dispatch the
> xforms-submit-error event?  Currently, Novell, fP and XSmiles all just show the
> response from the server and do NOT dispatch the error.

Not a lot of direct replies, but there seems to be agreement on that that we should throw the error on 4xx and 5xx for 1.0. Needs to be "enhanced", but that's post 1.0 stuff.

Comment 11

[Allan Beaufour]

Attachment: Patch

Comment 12

[Allan Beaufour]

Fixed on trunk