[patch] working out namespaces/css issues (was xml+css borken in head)

David Faure dfaure at klaralvdalens-datakonsult.se
Fri Oct 24 12:03:27 BST 2003


On Friday 24 October 2003 03:29, Germain Garand wrote:
> Hello,
> Some times ago, I said I'd work on those issues.
> 
> Here is broadly what the attached patch does:
> - bring back the NodeImpl_NSMask bits in id() for Attributes and XMLElements
>    => the cssselector need that to support CSS namespaces in an efficient way.
>    => fixes CSS selection for XML documents
> - change Nodeimpl::Id registration mechanism: 
>   Peter Kelly tried to solve the DOM1/DOM2 NS methods compatibility problems
>   by registering the qualifiedName. However, this is doomed to  fail in other
>   places that relies on Ids because prefixes aren't  encoded in  the Ids and 
>   are irrelevant for NS nodes identification.
>   Instead, I used registered aliases (see DocumentImpl::getId) to solve  
>   compatibility issues while maintaining a sound Id system.
> - support some more exceptions in dom/ (createElement(NS),
>   createAttribute(NS), create DocumentFragment ...)
> - factor qualifiedName exception checking code.
> - fix a bug in the xml tokenizer (startElement must use the qualifiedName, not
>   the localName, otherwise we don't get the prefix)
> - avoid scanning the qualifiedName 3 times
> 
> Do you think it's OK?

I like this approach better than what I have been doing in the 3.1 branch,
because you finally replaced those linear lists (well arrays, but used linearly)
with dicts. And both PMK and you factorized the code, compared to 3.1 branch.

I can see that the idea of fixing the branch and then porting to HEAD wasn't
as good as the idea of fixing HEAD directly like you did. The code is much
better now in HEAD (e.g. single getId()...). I'll drop my patch then.

What's excellent is that we all chose to add a bool to distinguish xhtml vs html-compat
in elements (to be fair Dirk told me to do so :)
I had put it in NodeImpl because I thought I was going to need it for attributes,
but your patch proves that it's not the case, so it's fine in HTMLElementImpl.

The only thing is that we chose m_htmlCompat instead of m_xhtml to make it
more obvious that html-compat is the legacy special case.
With that idea in mind, I changed htmltags.c to list tags lowercase instead 
of uppercase.

Another difference is that you assigned xhtml the namespace 1, whereas
in the branch I was still using 0 for it. But that makes sense I guess.
In my patch for the branch it wasn't necessary, because m_htmlCompat
would distinguish between html and xhtml elements anyway. Hmm so why do
you need it? Well of course code-wise it allows to do everything in getId().....
(in my patch I had removed the htmlMode-dependent case-sensitive comparisons
from tagId().

Did you forget to copy m_xhtml in cloneNode? This isn't covered by the testcases,
so I'm not sure. Same problems with importNode (which I don't understand why
it's not calling cloneNode). But that's something else.

Please commit, and if you agree I'll turn m_xhtml into m_htmlCompat (and
change htmltags.c to lowercase? Better move away from legacy stuff...)

-- 
David Faure -- faure at kde.org, dfaure at klaralvdalens-datakonsult.se
Qt/KDE/KOffice developer
Klarälvdalens Datakonsult AB, Platform-independent software solutions




More information about the kfm-devel mailing list