Skip to content

Incremental serializer: fix namespaces #79

Closed
wants to merge 5 commits into from

2 participants

@SimonSapin
  • Reject invalid prefixes (only None, unicode or ASCII-only bytes are valid)
  • Pick a prefix for namespaces without one
  • Write prefix declarations with attributes.
@scoder
lxml member

I don't think this is going to work. Just because you add the declaration to the (reversed) nsmap doesn't mean that the element that originally owned that particular nsmap wrote the declaration into the file. So subsequent siblings may end up with a missing prefix declaration. It should work to put it into the nsmap that was originally passed into this function, though.

_find_prefix() is only used in _write_start_element() and _write_attributes() and the latter is only used in the former, which in turn is only used when a element is being written to the file.

A declaration does not apply to siblings.

lxml member

What I meant was that this code is potentially changing the nsmap of an ancestor element instead of the local one. If that happens, a subsequent lookup from a sibling will pick up a prefix that is not declared in its scope.

Oh, I missed that. Of course this should only changed the dict being passed as a parameter. Switching now to a different variable name. Thanks!

@scoder
lxml member

This looks a bit, well, funny. I think namespace declarations should be written out explicitly as part of the element writing and should appear before the attributes (which might need them).

We can easily switch to serialize prefix declarations before "normal" attributes, but I don’t think it is supposed to change anything.

http://www.w3.org/TR/REC-xml-names/#scoping

The scope of a namespace declaration declaring a prefix extends from the beginning of the start-tag in which it appears to the end of the corresponding end-tag

And the syntax of a declaration really is the same as that of attributes.

lxml member

Sure. I'm fine with doing it that way, just switch the order.

@scoder
lxml member

Right, I mixed this up. Thanks for catching it.

Two things here: test if prefix is None rather than just if prefix (do not accept 0 or the empty string), and add the _prefixValidOrRaise check.

@scoder
lxml member
scoder commented Nov 22, 2012

Merged. Thanks!

@scoder scoder closed this Nov 22, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.