<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101857/">http://git.reviewboard.kde.org/r/101857/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 6th, 2011, 5:04 p.m., <b>Sebastian Sauer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thinking a bit more in my opinion we should not change how whitespaces are handled atm if there is no good reason for it (like a performance-boost or ...). The "2.10 White Space Handling" section from http://www.w3.org/TR/REC-xml/#sec-white-space makes it rather clear that;
<quote>
<!ATTLIST poem xml:space (default|preserve) 'preserve'>
The value "default" signals that applications' default white-space processing modes are acceptable for this element; the value "preserve" indicates the intent that applications preserve all the white space.
</quote>
What means that there are 2 modes. The default one (we are currently using) and the preserved one (which this patch tries to implement). Per ODF-specs the ODF XML-content is using the default but not the preserve mode (see e.g. http://blogs.oracle.com/lars/entry/whitespace_in_odf ).
So, this patch would result in us not proper handling ODF documents any longer :-/
</pre>
</blockquote>
<p>On August 6th, 2011, 9:23 p.m., <b>Jan Hambrecht</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think we have to differentiate between reading xml and reading odf. What i mean with that is not all xml is odf, and that class is an xml reader not an odf reader. So there are more uses cases than just reading odf. Svg reading is one of them. And preserving whitespace when reading svg text is a required feature.</pre>
</blockquote>
<p>On August 7th, 2011, 4:25 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Maybe then it would be a good idea to not always do it but add a flag with that it is possible to preserve the spaces. As a default the current way should be used. And the flag can be set when spaces should be preserved. WIth that both is possible.</pre>
</blockquote>
<p>On August 16th, 2011, 11:54 a.m., <b>Sebastian Sauer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for the explanations and suggestions Jan and Thorsten and lot of thanks to Jaime for implementing that. Amazing work! The patch looks really good (as in perfect) to me and imho we can "ship it" if none else has feedback / sees problems with the updated patch?
</pre>
</blockquote>
<p>On August 16th, 2011, 11:57 a.m., <b>Thorsten Zachmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I would like to run cstester to make sure there are no regressions. As I'm traveling today I hope I can do it tomorrow</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">cstester showed no problems, so fine with me after you implemented the changes jan propsed.</pre>
<br />
<p>- Thorsten</p>
<br />
<p>On August 18th, 2011, 12:04 a.m., Jaime Torres Amate wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Jaime Torres Amate.</div>
<p style="color: grey;"><i>Updated Aug. 18, 2011, 12:04 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 have added the TestXmlReaderWithoutSpaces to remove all the additional spaces between nodes.
The second version of this patch introduces a new method that is called only when setContent is called with the new default boolean parameter set to false.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The TestXmlReader test and the new TestXmlReaderWithoutSpaces are OK.
Today, only the following tests failed:
19 - Plan-ScriptingTester (Failed)
20 - KPlato-RCPSTester (Failed) (with a crash)
24 - kspread-Formula (Failed)
37 - kspread-InformationFunctions (Failed)
41 - kspread-TextFunctions (Failed)
43 - kspread-ValueFormatter (Failed)
172 - libs-widgets-KoResourceTagging_test (Failed)
193 - libs-kotext-styles-TestOpenDocumentStyle (Failed)
196 - kotext-odf-TestChangeTracking (Failed)
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/odf/KoXmlReader.h <span style="color: grey">(3f9ddf4)</span></li>
<li>libs/odf/KoXmlReader.cpp <span style="color: grey">(ad5e9d2)</span></li>
<li>libs/odf/tests/CMakeLists.txt <span style="color: grey">(ad632c8)</span></li>
<li>libs/odf/tests/TestXmlReaderWithoutSpaces.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101857/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>