Review Request: read XML in the rigth way. ie. <a> <b>\n<c> has 5 nodes, not 3

[Sebastian Sauer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101857/#review5440
-----------------------------------------------------------


1. Please test loading with ODT/ODS/ODP files produced with Words/Tables/Stage. OO.org does save all the content in one line and therefore has no additional spaces in front. For me it looks like those produced documents would fail now cause we are handling spaces wrong (that's also why the unittests are failing).

2. Note that it's perfect legal in ODF to have tons of spaces in front. Per specs multiple spaces are compressed to one space. To preserve spaces text:s elements need to be used. I fail to see why we now need to preserve spaces if they *NEED* to be thrown away again later.

3. I fear that this has in impact on performance cause now we handle spaces which we just skipped before (and later throw them away again). So, even if my point 1. above isn't true then we still need to analyse performance before vs after. So, it would be useful if you can remove debug-code from the patch like those "node->dump();"


- Sebastian


On Aug. 6, 2011, 10:56 a.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101857/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2011, 10:56 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Summary
> -------
> 
> Quoting the w3 consortium:
> [Definition: All text that is not markup constitutes the character data of the document.]
> 
> And in section
> http://www.w3.org/TR/REC-xml/#sec-white-space
> 
> In editing XML documents, it is often convenient to use "white space" (spaces, tabs, and blank lines) to set apart the markup for greater readability. 
> Such white space is typically not intended for inclusion in the delivered version of the document. 
> On the other hand, "significant" white space that should be preserved in the delivered version is common, for example in poetry and source code.
> 
> An XML processor MUST always pass all characters in a document that are not markup through to the application. 
> A  validating XML processor MUST also inform the application which of these characters constitute white space appearing in element content.
> 
> [Definition: An element type has mixed content when elements of that type may contain character data, optionally interspersed with child elements.] 
> ------------------
> The attached patch modifies the xml parser to return the spaces between > and < as text elements.
> 
> I needed to change the TestXmlReader to remove all the additional spaces between nodes.
> (I'll need to modify the patch to remove all the additional spaces I've introduced).
> 
> 
> Diffs
> -----
> 
>   libs/odf/KoXmlReader.cpp ad5e9d2 
>   libs/odf/KoXmlReaderForward.h 4ca9a74 
>   libs/odf/tests/TestXmlReader.cpp 6631b64 
> 
> Diff: http://git.reviewboard.kde.org/r/101857/diff
> 
> 
> Testing
> -------
> 
> The modified TestXmlReader test is OK.
> There are only 2 regressions in the tests that I do not know how to fix:
>         147 - krita-ui-KisKraLoaderTest (Failed)
>         148 - krita-ui-KisKraSaverTest (Failed)
> 
> Also, I've been able to read with calligrawords and calligrastage all the .od* that I have without problems.
> 
> 
> Thanks,
> 
> Jaime Torres
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110806/d34268b5/attachment.htm>