344621 - Instance docs with doc-level comments and processing instructions are stripped during submission

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Steve Speicher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

Other processors (XSmiles and formsPlayer) perserve comments and processing instructions when submitting documents.  The spec says that all comments and processing instructions should be preserved [1] but as the example shows, when the node bound to is '/' does it refer to the element node or the document node and are pi's and comment nodes considered descendants [2] of it.  I have sent a note to the WG for clarification [3].

I recommend that the behavior be changed to match XSmiles/formsPlayer as it appears to be what makes sense.  If there is a need to change it based on WG reply, then a separate bug should be filed.

Take for example an external XML instance document that has:
001:  <?xml version="1.0" encoding="UTF-8" standalone="no"?>
002:  <?xml-stylesheet type="text/xsl" href="toHtml.xsl"?>
003:  <!-- Please save my important document comments -->
004:  <data>
005:    <!-- Please save my important nested comments -->
006:    Stuff
007:    <!-- Please save my important nested comments -->
008:  </data>

After saving/submitting this it looks like:
001:  <?xml version="1.0" encoding="UTF-8" standalone="no"?>
004:  <data>
005:    <!-- Please save my important nested comments -->
006:    Stuff
007:    <!-- Please save my important nested comments -->
008:  </data>

[1] http://www.w3.org/TR/xforms/index-all.html#structure-model-instance
[2] http://www.w3.org/TR/xforms/index-all.html#structure-model-submission
[3] http://lists.w3.org/Archives/Public/www-forms/2006Jul/0021.html

Reproducible: Always

Steps to Reproduce:
1. extract test cases locally to the same folder
2. open XHTML/XForms file simple.xhtml
3. press 'Save' button
4. observe differences in data.xml

Comment 1

[Steve Speicher]

Attachment: driver XForms

Comment 2

[Steve Speicher]

Attachment: Instance data for test form

Comment 3

[Steve Speicher]

Attachment: patch that adds source doc nodes

Note, was wrong about XSmiles...it doesn't perserve pis and comments, only formsPlayer does.  formsPlayer doesn't preserive them when the submission is bound to something other than that root document element, this patch behaves that way too.

Comment 4

[alexander :surkov (:asurkov)]

Does your patch preserve comments that aren't direct children of document root?

Comment 5

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> Does your patch preserve comments that aren't direct children of document root?
> 

I guess bug name answers on my question :). But should such comments be preserved?

Comment 6

[Steve Speicher]

(In reply to comment #5)
> (In reply to comment #4)
> > Does your patch preserve comments that aren't direct children of document root?
> > 
> 
> I guess bug name answers on my question :). But should such comments be
> preserved?
> 

What I have run into personally and have heard others with similar problems is, that they want to take an existing instance document (which happens to have doc-level comments and pi's) and utilize XForms to modify the instance.  Because of limitations that this patch removes, it is not possible for this scenario to be supported.

I think that they should be preserved, perhaps the XForms WG might propose a solution that adds an attribute to the submission element, say:
  <submission omit-doc-level-nodes="yes" ... >

I think what this patch supports should be sufficient.

Comment 7

[Doron Rosenberg (IBM)]

Comment on attachment 229291 [details] [diff] [review]
patch that adds source doc nodes

Patch looks good

Comment 8

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

Comment on attachment 229291 [details] [diff] [review]
patch that adds source doc nodes

What happens to PIs and comments which are after documentElement in the original document?

Comment 9

[Steve Speicher]

(In reply to comment #8)
> (From update of attachment 229291 [details] [diff] [review] [edit])
> What happens to PIs and comments which are after documentElement in the
> original document?

They are preserved just fine since the serialization code only would import nodes starting with descendents of the root document element. (can see if running the supplied test case as-is, since it has comments mixed throughout the nodes)  This patch iterates over the child nodes of the document element itself.

Comment 10

[Steve Speicher]

Attachment: patch that preverses pis and comments before AND after documentElement

Comment 11

[aaronr]

looks like we'll fix it for compatibility/common sense reasons.  Still no word back from the W3C

Comment 12

[aaronr]

Looks like you blindly copy all children of the submission document.  What would happen if the submission document isn't well-formed XML anymore?  For example, what if there are two root-level elements?  What does formsPlayer do in that case?  Do they only copy over sibling comments and PI's or all sibling nodes of the root?

Comment 13

[Steve Speicher]

(In reply to comment #12)
> Looks like you blindly copy all children of the submission document.  What
> would happen if the submission document isn't well-formed XML anymore?  For
> example, what if there are two root-level elements?  What does formsPlayer do
> in that case? 

I'm not sure how it can get into this situation as Mozilla's DOM implementation forbids it, see DOMNode.InsertBefore() http://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#ID-952280727

I did a bit of testing and get assertion in Moz and similar problems/errors in formsPlayer when even attempting to create 2 documentElements, either declaratively or scripting.

So I could test for such a case if desired but appears it *should* never occur.  It would just add a little bit of overhead having said that.

> Do they only copy over sibling comments and PI's or all sibling
> nodes of the root?
>
formsPlayer copies over doc-level and all descendant pis and comment nodes, so this patch behaves exactly like formsPlayer.

Comment 14

[aaronr]

I couldn't make it break

Comment 15

[aaronr]

checked into trunk for sspeiche

Comment 16

[aaronr]

checked into 1.8.0 branch on 2006/09/21

Comment 17

[aaronr]

checked into 1.8 branch on 2006/11/21