> On Aug. 6, 2011, 4:35 p.m., Sebastian Sauer wrote: > > 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();" > >
Guess I wasn't clear enough with point 2; spaces at the beginning and end are *always* removed. Multiple spaces between text are compressed to one space. That means that something like "<text:p> Hello World </text:p>" becomes "<text:p>Hello World </text:p>" in ODF. See here the normalizeWhitespace function in calligra/libs/libs/kotext/opendocument/KoTextLoader_p.h. That code-path is performance-critical. - Sebastian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101857/#review5440 ----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
