Opened 10 years ago

Closed 9 years ago

Last modified 9 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: Launchpad Bug:

Description


Change History (12)

comment:1 Changed 10 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 10 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 9 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 9 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 9 years ago by ralphm

  • Priority changed from normal to highest

comment:6 Changed 9 years ago by ralphm

  • Owner ralphm deleted

comment:7 Changed 9 years ago by warner

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

comment:8 Changed 9 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 9 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 9 years ago by radix

  • Milestone changed from Twisted-2.3 to Twisted-2.4

Milestone Twisted-2.3 deleted

comment:11 Changed 9 years ago by ralphm

  • Milestone changed from Twisted-2.4 to Words-0.4

comment:12 Changed 4 years ago by <automation>

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