[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: Binary incompatible changes: khtml
From:       Peter Kelly <pmk () post ! com>
Date:       2000-05-30 12:25:49
[Download RAW message or body]

Lars Knoll wrote:
> Hi,
> 
> sounds mostly fine to me. I have one comment though.
> 
> On Mon, 29 May 2000, Peter Kelly wrote:
> > I'm assuming that this wednesday is the last chance to make binary
> > incomatible chanes, sot here's a few things I need to clean up with
> > khtml. These are changes that will only affect apps that access khtml's
> > DOM tree (Kafka is the only one I know of).
> > 
> > Briefly, these include:
> > - Some classes that are not descended from Node do not have Impl
> > classes, or have Impl classes of the wrong type (e.g. DOMImplementation,
> > DocumentRange, NamedNodeMap)
> 
> There is a reason for this. DOMImplementation for example is a class with
> no functions one can write to. So I don't see the reson for havin an Impl
> vclass there. If I remember correctly (no CVS access from here),
> NamedNodeMap is one of the classes you say has a pointer to an Impl class
> of the wrong type (a pointer to a nodeImpl). But this is basically what is
> needed for the class to work correctly, so I'm not sure if this really
> needs to be changed. Anyway, changing a pointer later on to point to
> something different gives no problems for binary compatibility.

This is mainly for flexibility - although there are no writable
attributes in DOMImplementation at the moment, it's not inconcievable
that the W3C may add some in later versions of the DOM spec (although
this is perhaps less likely in DOMImplementation as some of the other
classes), and if they do it will save us some headaches if we do need an
impl. I think the small tradeoff of the extra memory for a pointer is
worth it to avoid some potentially messy coding in the future ;)

Also, in the case of all (or at least, most) object types, the
ECMAScript bindings need to know whether or not they are null or not so
they can return a Null ECMAScript type instead of the node (so you can
do == null, etc.). So the impl comes in here as well.

As far as NamedNodeMap is concerned, this is also for flexibility, but
in this case I found that I need it for the current implementation.
Previously, in the case of Elements, the NamedNodeMap functions called
the appropriate methods in Element like getAttributeNode(). But I have
recently changed that implementation so that a real NamedNodeMap is
present for each Element (which btw, also uses real Attr object). These
are used instead of Attribute and AttributeList now (at least for
storing values after parsing is complete). The reason behind this is
that getAttributeNode() has to return the same attribute node every
time, otherwise image.getAttributeNode("src") ==
image.getAttributeNode("src") would be false, which is wrong.

So the upshot of the above is that we can't use just a NodeImpl for the
NamedNodeMap - we need our own class. I already made these changes a
while ago, but I've waited till now to make the API change. I can see
though that your comment about changing types of pointers makes sense.

> 
> > - All the classes in dom2_range.h and dom2_traversal.cpp do not have
> > impl classes, and also have some internal variables in the main class.
> > These should be moved to impl classes (and also the processing code for
> > these classes)
> 
> For these classes I agree.
> 
> > - Some classes do not have an isNull() method or handle() method (to get
> > access to the impl class).... these are needed for ECMAScript to do type
> > conversion and equality properly (actually, these may not cause BCI
> > problems)
> 
> Just add them.
> 
> > - Node::applyChanges() and Node::getCursor() to not need to be virtual
> > (the impl classes handle the virtual stuff)
> > 
> > Any objections me committing these changes?
> 
> Just take 5 minutes to think about my comment above, otherwise go ahead.
> 
> Lars
> --
> taking half an hour break from his holidays to answer some mails...


-- 
Peter Kelly
pmk@post.com

-- 
Peter Kelly
pmk@post.com

[prev in list] [next in list] [prev in thread] [next in thread]