280966 - submit get terminated with extra separator

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[aaronr]

Reported by beta1 tester:

"It looks like in submission method="get" that the separator character is being
used as a terminator.
This is an erratum that should already have been issued.  The goal is HTML 4
compatibility."

I didn't find it in the erratum, but it makes sense.

Using the testcase noted in the URL field, the actual result will look like:
http://xformstest.org/cgi-bin/echo.sh?a=1&b=2&

Using Novell's pluin, the result looks like:
http://xformstest.org/cgi-bin/echo.sh?a=1&b=2

Comment 1

[Doron Rosenberg (IBM)]

Attachment: strip '&' if its the last character - easy simple hack.

Comment 2

[Doron Rosenberg (IBM)]

Quick fix.  Seems easier to just do this rathar than modify the submission
recursion code.

Comment 3

[Leigh L. Klotz, Jr.]

Looks good but it needs to trim separator, instead ofa constant '&', because by
default separator is ';' but can be set to '&'.  See the enumeration of
submission/@separator in the Schema.  (While you're there you might want to
verify that it doesn't add a '?' if one is already there.)

Comment 4

[Doron Rosenberg (IBM)]

so what should happen if the separator is "eeeeek" - should it fail or default
to ";"?

Comment 5

[Leigh L. Klotz, Jr.]

If the separator is not ';' or '&', it is an authoring error, as the document is
invalid by the XForms Schema (which is a normative part of the definition of
XForms).  The processor should act as if the error were caught by an XML Schema
validation of the page, even if it's not using a validating XML parser.

As an author, I would expect to see the XML error page with the pretty colored
arrows pointing to the submission attribute value.  If you do that, users will
never see the error, as the page simply won't load for the authors.  

It's the same as if someone had put anything else illegal in the namespace, such
as <xforms:fnord/> .

Comment 6

[Doron Rosenberg (IBM)]

Novell's plugin for IE does indeed follow this hidden errata, but it does allow
any type of separator.

Comment 7

[Leigh L. Klotz, Jr.]

Here are normative references for the limitation to ; and &:
[1] http://www.w3.org/TR/xforms/index-all.html#structure-abstract 
From the content model for the XForms Core Module:
   separator (';' | '&')

[2] http://www.w3.org/TR/xforms/index-all.html#schema
From the Schema:
<xsd:attribute name="separator" use="optional" default=";">
 <xsd:simpleType>
  <xsd:restriction base="xsd:string">
   <xsd:enumeration value=";"/>
   <xsd:enumeration value="&"/>
  </xsd:restriction>
 </xsd:simpleType>
</xsd:attribute>

Comment 8

[Doron Rosenberg (IBM)]

Attachment: Lastest work (this converts invalid separators to ';' for now, which is against the spec)

I am aware Novell is wrong :)

This patch will convert invalid separators to ';', since we don't have a way to
say "invalid xforms, show a error page" yet.  Once that works, we can switch
this to not care abot invalid ones.

Comment 9

[Leigh L. Klotz, Jr.]

The erratum is now public:
http://www.w3.org/2003/10/REC-xforms-10-20031014-errata.html#E62 

Comment 10

[Doron Rosenberg (IBM)]

Attachment: newer patch

This patch does:
  - strip the separator from the end
  - if the separator is not ; or &, we abort submission and report the error
(into the JS console for now).

Comment 11

[aaronr]

Comment on attachment 189101 [details] [diff] [review]
newer patch

make sure when you check in that you don't have the alignment problems in
SerializeData that this patch seems to show.

Also this patch is missing the .properties file.

with those, r=me

Comment 12

[Leigh L. Klotz, Jr.]

(In reply to comment #10)
>   - if the separator is not ; or &, we abort submission and report the error
> (into the JS console for now).
Great!  

One possible twiddle: if you can swing it, reporting this (and similar
validation errors) in the various elements' OnCreated methods might be better,
if possible, because it will show up as an authoring error (which it is) rather
than a runtime error. I can understand if Mozilla policy prohibits this, but
that would make it act as if the page were read with a validating schema parser,
which would catch it when reading the page.  

DOM access would then be an open question though (what happens if someone gets
the XHTML page's DOM and changes @submission?) so I can understand holding off
on it for that point as well.

Comment 13

[Doron Rosenberg (IBM)]

(In reply to comment #12)
> (In reply to comment #10)
> >   - if the separator is not ; or &, we abort submission and report the error
> > (into the JS console for now).
> Great!  
> 
> One possible twiddle: if you can swing it, reporting this (and similar
> validation errors) in the various elements' OnCreated methods might be better,
> if possible, because it will show up as an authoring error (which it is) rather
> than a runtime error. I can understand if Mozilla policy prohibits this, but
> that would make it act as if the page were read with a validating schema parser,
> which would catch it when reading the page.  
> 
> DOM access would then be an open question though (what happens if someone gets
> the XHTML page's DOM and changes @submission?) so I can understand holding off
> on it for that point as well.
> 
> 
> 

Should an xforms event be thrown when submission is aborted?

Comment 14

[Doron Rosenberg (IBM)]

Attachment: added the missing properties change

Comment 15

[Leigh L. Klotz, Jr.]

> Should an xforms event be thrown when submission is aborted?

xforms-submit-error would seem to fit, but it was designed for the server to
report errors back, for not reporting authoring errors, since presumably the
author should have taken care of those in authoring, not in error handling code.
 As far as the spec is concerned, the page itself is invalid and shouldn't even
load, so there's no way for a user to get far enough to press the button.  So,
if you want to minimize work on this bug and just move forward, it would be
"fatal error processing stops" as there is no chance of recovery anyway.

Comment 16

[Doron Rosenberg (IBM)]

checked in.