Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#1444 defect closed fixed (fixed)

twisted.words.xish.domish doesn't deal with namespaces properly

Reported by: ralphm Owned by:
Priority: high Milestone: Words-0.4
Component: words Keywords:
Cc: ralphm, radix Branch:
Author: Launchpad Bug:

Description


Change History (9)

comment:1 Changed 9 years ago by ralphm

xish has several issues with namespaces that yield incorrect serializations:

 1. The default namespace is not reflected in serialization if the element has
    no parent
 2. Using 'no' namespace via xmlns='' doesn't work
 3. Prefix declarations (xmlns:*) of namespaced attributes are not generated

ad 1. This comes from Xish' Jabber specific roots.

ad 2. In the current implementation, there is no difference between not
specifying a namespace and specifying the empty namespace on Element creation.

comment:2 Changed 9 years ago by ralphm

  • Keywords review added; words removed
  • Milestone set to Twisted 2.3

These issues have been fixed in source:branches/domish-namespaces-1444-2.

comment:3 Changed 9 years ago by ralphm

  • Cc radix added
  • Owner changed from ralphm to radix
  • Status changed from assigned to new

comment:4 Changed 9 years ago by radix

  • Keywords review removed
  • Owner changed from radix to ralphm

Ok

  • I'm not very happy about the way test_domish is done. Generally, one should try not to put assertions in methods that aren't directly and specifically called by the unit test method. Specifically I'm referring to the assertions about matched elements and such in _docStarted, _elementMatched, and so forth. A bug which prevents these methods from being called or completed could make the test look OK. But, I know that this is how it was before you came to it, so it's not required to be fixed for this merge.
  • As I mentioned on IRC, generateOnlyInterface can be replaced with reduce(IFace.providedBy, list)
  • "raise AttributeError" in getattr shoudl be "raise AttributeError(key)"

Good docs!

Go ahead and merge after the AttributeError is improved.

comment:5 Changed 9 years ago by ralphm

  • Status changed from new to assigned

About the tests in DomishStreamTests, yes, I'm not too happy about those, either. Also, it'd be better to query the generated objects directly, instead of relying on the XPath stuff. Be sure that it will be addressed when I need add new tests there.

Assuming you meant filter instead of reduce. As there are other generate* functions in domish.py, too, I'll just leave it for now. Also, filter() does not return a generator, but a constructed list.

Fixed the AttributeError stuff in r16634.

Thanks!

comment:6 Changed 9 years ago by ralphm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [16635]) Fix namespace issues in domish and add IElement

Author: ralphm
Reviewer: radix
Fixes #1444

  • Make domish handle prefixed attributes properly
  • Allow the use of the empty namespace
  • Some other edgecases concerning namespaces
  • Let Element.toXml() take a defaultUri parameter for partial serializations.
  • Allow manually constructed Elements to take None as a namespace, meaning it inherits the default namespace from its (logical) parent.
  • Parsing XML using elementStream().parse() will always yield Elements with defined namespaces.
  • Add the IElement interface and check objects using providedBy instead of using isinstance.
  • Add lots of documentation

comment:7 Changed 9 years ago by radix

  • Milestone changed from Twisted-2.3 to Twisted-2.4

Milestone Twisted-2.3 deleted

comment:8 Changed 9 years ago by ralphm

  • Milestone changed from Twisted-2.4 to Words-0.4

comment:9 Changed 4 years ago by <automation>

  • Owner ralphm deleted
Note: See TracTickets for help on using tickets.