New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sxml: fix ssax parser to trim whitespace according to xml:space #360

Merged
merged 1 commit into from Jul 10, 2018

Conversation

Projects
None yet
2 participants
@pclouds
Copy link
Contributor

pclouds commented Jul 4, 2018

XML standard [1] specifies that if a tag has xml:space="preserve" and it only contains whitespaces then all the whitespaces should be preserved.

The current SSAX parser does parse and pass xml:space around (child nodes inherit this attribute from the parent node) but it does not actually follow this rule.

Fix this by checking for the presence of xml:space then decide whether to drop whitespaces. The fix is not perfect, we should use preserve-ws? that's propagated throughout the nodes. Unfortunately we
don't have access to it at this point of dropping whitespace.

[1] https://www.w3.org/TR/xml/#sec-white-space

sxml: fix ssax parser to trim whitespace according to xml:space
XML standard [1] specifies that if a tag has xml:space="preserve" and
it only contains whitespaces then all the whitespaces should be
preserved.

The current SSAX parser does parse and pass xml:space around (child
nodes inherit this attribute from the parent node) but it does not
actually follow this rule.

Fix this by checking for the presence of xml:space then decide whether
to drop whitespaces. The fix is not perfect, we should use
preserve-ws? that's propagated throughout the nodes. Unfortunately we
don't have access to it at this point of dropping whitespace.

[1] https://www.w3.org/TR/xml/#sec-white-space
@shirok

This comment has been minimized.

Copy link
Owner

shirok commented Jul 10, 2018

Just to clarify: This is applying the upstream fix, right?

@shirok shirok merged commit c92d445 into shirok:master Jul 10, 2018

shirok added a commit that referenced this pull request Jul 10, 2018

@pclouds

This comment has been minimized.

Copy link
Contributor

pclouds commented Jul 10, 2018

Just to clarify: This is applying the upstream fix, right?

I'm not even sure if sxml upstream is still active anymore (at least the sourceforge page does not indicate any activity except packaging for other scheme implementations).

@shirok

This comment has been minimized.

Copy link
Owner

shirok commented Jul 10, 2018

Neither do I. But I see that your fix seemed to be in the sf.net repo as well. Just wanted to be aware of how our fix syncs the other implementations. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment