<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 July 5th, 2011, 5:18 p.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;">For me that looks quite wrong. The tests you changed should work even if there is a space at the beginning of the line. That is definitely something that needs very deep testing fotr that I need some time to see it does not break stuff. But for me it looks like that it will break stuff.</pre>
</blockquote>
<p>On July 5th, 2011, 5:20 p.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;">Here is part of the ODF spec that might be relevant for that change:
6.1.2 White space Characters
Consumers shall collapse white space characters that occur in
? a <text:p> or <text:h> element (so called paragraph elements), and
? in their descendant elements, if the OpenDocument schema permits the inclusion of character
data for the element itself and all its ancestor elements up to the paragraph element.
Collapsing white space characters is defined by the following algorithm:
1) The following [UNICODE] characters are normalized to a SPACE character:
?HORIZONTAL TABULATION (U+0009)
?CARRIAGE RETURN (U+000D)
?LINE FEED (U+000A)
?SPACE (U+0020)
2) The character data of the paragraph element and of all descendant elements for which the
OpenDocument schema permits the inclusion of character data for the element itself and all its
ancestor elements up to the paragraph element, is concatenated in document order.
3) Leading SPACE characters at the start of the resulting text and trailing SPACE characters at the
end of the resulting text are removed.
4) Sequences of SPACE characters are replaced by a single SPACE character.
</pre>
</blockquote>
<p>On July 5th, 2011, 5:27 p.m., <b>Jaime Torres Amate</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;">Then I'll discard it. I did not know that part of ODF spec. Long time ago I only read the schema (in graphical mode).
I'll create a mini-patch to remove the
QEXPECT_FAIL("", "Whitespace handling should be fixed.", Continue);
in the TestXMLReader test.
</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;">This has nothing to do with odf. KoXmlReader is not a consumer, the application is. So collapsing whitespace at the xml reader level is wrong, imho. And I can tell you that the missing data did bite me when implementing svg text support. So I am in favour of correcting that misbehaviour.</pre>
<br />
<p>- Jan</p>
<br />
<p>On July 5th, 2011, 4:37 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 July 5, 2011, 4:37 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 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).</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 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.
</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.cpp <span style="color: grey">(ad5e9d2)</span></li>
<li>libs/odf/KoXmlReaderForward.h <span style="color: grey">(4ca9a74)</span></li>
<li>libs/odf/tests/TestXmlReader.cpp <span style="color: grey">(6631b64)</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>