<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 16th, 2011, 5:45 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;">Instead of passing a new bool parameter to all these methods of KoXmlReader, I would prefer having a single function setWhitespaceStrippingEnabled. This sets an internal flag which can be used at all the places necessary.</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;">There are involved 1 class (KoXmlDocument) and 2 namespaces (KoXml -public- and anonymous -private-) in the patch.
I do not like the idea of having one global variable in each of the namespaces and in the class (or a module global variable) that are modified by one method in the class (or in the KoXml namespace).
I do not like the idea of a single function (outside of the class) changing the behavior of a class.
I do not like the idea of a class method changing the behavior of a namespace.
I would have done it if all the methods were in one class or in children classes or in private namespaces.
</pre>
<br />
<p>- Jaime Torres</p>
<br />
<p>On August 15th, 2011, 5:56 p.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. 15, 2011, 5:56 p.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 modified 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>