Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#1074 defect closed fixed (fixed)

domish.Element checking __class__, breaks inheritance

Reported by: VislabSteve Owned by:
Priority: highest Milestone: Words-0.4
Component: words Keywords:
Cc: ralphm, VislabSteve Branch:
Author:

Description


Change History (12)

comment:1 Changed 12 years ago by VislabSteve

twisted.xish.domish.Element.addChild checks the class type for setting the parent:
    if node.__class__ == Element:
            node.parent = self
However this breaks when node is a subclass of Element (i.e. inherits from it).
 isinstance(node, Element) would be a better method.

Note there are other places that make this comparison: firstChildElement() and
__getattr__()

Thanks

comment:2 Changed 11 years ago by ralphm

Confirmed. It would be even better if we worked with interfaces here, so that we
can avoid the isinstance horrors. I'll look into this.

comment:3 Changed 11 years ago by ralphm

  • Component set to words
  • Keywords words removed
  • Milestone set to Twisted 2.3
  • Priority changed from high to normal
  • Status changed from new to assigned

source:branches/domish-namespaces-1444-2 adds a domish.IElement to remedy this. When that branch has been merged I'll review the rest of the code to use it.

comment:4 Changed 11 years ago by ralphm

  • Keywords review added
  • Status changed from assigned to new

As #1444 has been solved, all that remained was replacing isinstance calls. Created source:branches/xish-IElement-1074 and did that.

comment:5 Changed 11 years ago by ralphm

  • Priority changed from normal to highest

comment:6 Changed 11 years ago by ralphm

  • Owner ralphm deleted

comment:7 Changed 11 years ago by warner

  • Owner set to warner
  • Status changed from new to assigned

comment:8 Changed 11 years ago by warner

  • Keywords review removed
  • Owner changed from warner to ralphm
  • Status changed from assigned to new

looks good to me, handing back to ralphm for committing

comment:9 Changed 11 years ago by ralphm

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

(In [16693]) Remove last occurance of isinstance on Element objects

Author: ralphm Reviewer: warner Fixes #1074

comment:10 Changed 11 years ago by radix

  • Milestone changed from Twisted-2.3 to Twisted-2.4

Milestone Twisted-2.3 deleted

comment:11 Changed 11 years ago by ralphm

  • Milestone changed from Twisted-2.4 to Words-0.4

comment:12 Changed 6 years ago by <automation>

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