Issue 120334 - svg: fill color is rendered wrongly
svg: fill color is rendered wrongly
Status: VERIFIED FIXED
Product: Draw
Classification: Application
Component: open-import
3.4.0
All All
: P3 normal (vote)
: 4.0.0
Assigned To: Armin Le Grand
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-20 18:27 UTC by Regina Henschel
Modified: 2013-07-12 16:18 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---


Attachments
picture with node (4.15 KB, image/svg+xml)
2012-07-20 18:27 UTC, Regina Henschel
no flags Details
picture without node (3.29 KB, image/svg+xml)
2012-07-20 18:27 UTC, Regina Henschel
no flags Details
File that contains only the failing part (524 bytes, image/svg+xml)
2012-07-21 11:43 UTC, Regina Henschel
no flags Details
tab and carriage return delimiter (538 bytes, image/svg+xml)
2012-07-25 12:25 UTC, Regina Henschel
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description Regina Henschel 2012-07-20 18:27:13 UTC
Created attachment 78696 [details]
picture with node

The attached object "8050 manually reduced.svg" has as first element a path, which should have a white filling, but it is not white in AOO.
The attached object "8050 manually reduced without background.svg" is nearly the same, only the first node in the group is removed.

Open the files in a browser to see, how they should look. Then insert the pictures into a Draw document. The wrong color is obvious.

Both files are valid in W3C validator.
Comment 1 Regina Henschel 2012-07-20 18:27:51 UTC
Created attachment 78697 [details]
picture without node
Comment 2 Regina Henschel 2012-07-21 11:43:34 UTC
Created attachment 78699 [details]
File that contains only the failing part

The reason is, that the class attribute does not contain a single class, but a list. This is allowed, see http://www.w3.org/TR/SVG10/styling.html#ClassAttribute
Comment 3 Armin Le Grand 2012-07-22 11:49:10 UTC
ALG: Taking hownership
Comment 4 Armin Le Grand 2012-07-24 09:50:15 UTC
ALG: Regina, many thanks for finding this, I'll have a look.
Comment 5 Armin Le Grand 2012-07-24 12:39:06 UTC
ALG: Adapted CssStyle for SvgStyleAttributes to be a vector, not only a single entry. Adapted parser to set more than one entry. Moved some bools out of the way as preparation for changing getParentStyle() to a pair of getParentStyleCount()/getParentStyle(sal_uInt32 nIndex), need to determine closely for each attribute if it is set in this style or not.
Comment 6 Armin Le Grand 2012-07-25 09:07:00 UTC
ALG: Moved checkForCssStyle from SvgStyleAttributes to SvgNode, list of CSS styles is processed there. Modified to directly return a main StyleSheet, so when CSS are used it will create a modified temporary list used to create the primitives. Added a new proxy mechanuism to SvgStyleAttributes to support a temporary tree structure and use it.
Works as expected, testing my set of SVG test files to see if nothing is broken...
Comment 7 SVN Robot 2012-07-25 09:29:09 UTC
"alg" committed SVN revision 1365501 into trunk:
#120334# Added support for multiple CSS styles per graphic primitive in SVG
Comment 8 Armin Le Grand 2012-07-25 09:30:33 UTC
ALG: One more problem with fill rule corrected, looks good. Adding to trunk as revision 1365501.
@Regina: Please check this and do some tests (as soon as versions are available from the buildbots or build a current one). Thanks again for finding this.
Comment 9 Regina Henschel 2012-07-25 12:25:18 UTC
Created attachment 78723 [details]
tab and carriage return delimiter

Hi Armin, I'm not sure about the use of skip_char with character ' '. The list is white-space separated. It seems white space contains at least "space" (U+0020), "tab" (U+0009), "line feed" (U+000A) and "carriage return" (U+000D). I'm not sure about page break. See http://www.w3.org/TR/SVG/types.html section <list-of-strings>.
I have tried to find a formal definition of the attribute 'class', but have not found it.

I have attached a file with tab and CR delimiter instead of blank. The file is valid and shows fine in my Seamonkey.

BTW: Such files with multiple classes are generated by Corel Draw 12.
Comment 10 Armin Le Grand 2012-07-25 14:08:03 UTC
ALG @Regina: Not sure what you mean here. Just tried the new attachment with Firefox and my AOO with the made fix, both show the same. Both show a green rectangle with red border. Please add more info when you think something is going wrong. Thanks!
Comment 11 Regina Henschel 2012-07-25 15:31:15 UTC
@Armin: If it is OK with the patch, then it is fine. I have currently no build to test it and therefore thought it might be useful to attach such a file. My question was, whether you have considered other kind of "white-space" than blanks. And obviously you have :)

Do you handle tabs, for example, somewhere earlier in the import?
Comment 12 Armin Le Grand 2012-07-25 15:52:25 UTC
ALG @Regina: If you talk about whitespace in the SVG itself, I do not handle it myself, but implemented a com::sun::star::xml::sax::XDocumentHandler which has a ignorableWhitespace(..) call (see svgdocumenthandler.cxx|hxx in svgio module). The OO's XML import tools do the parsing and call the methods at XDocumentHandler. If you talk about whitespace inside strings, whiteSpaceHandlingDefault/whiteSpaceHandlingPreserve is used (both in svgio module) which handle newline, tab and space as whitespace. They much SVG whitespaces as defined in SVG1.2 somewhere (see http://www.w3.org/TR/SVG/text.html, there 'Xml character data in the instruction, leading to http://www.w3.org/TR/2008/REC-xml-20081126/#syntax, see 2.10). Whitespace can be preserved or not (hence my two methods above and their names) and I do that as defined. HTH!
Comment 13 Armin Le Grand 2012-07-25 16:08:01 UTC
ALG: @Regina: Here is the recipie used, derived from the definition: https://developer.mozilla.org/en/SVG/Attribute/xml%3Aspace. This was tricky, but needed for exactly supporting the SVG text examples (see all examples in http://www.w3.org/TR/SVG/text.html). Esp tspan with multiple X-Values and text on curve are hard, but all done :-)
Comment 14 Regina Henschel 2012-07-25 22:49:15 UTC
Hi Armin, thanks for your explanation. It is much clearer now for me.
Comment 15 Regina Henschel 2012-08-01 15:34:28 UTC
Verified in debug build of r1367616.

The original file, which I got from Corel Draw 12 is correctly rendered now.

But that file gives an error message. Because the reduced files, which are attached to this issue give no errors, I consider this a different bug and open a new issue for it.