CSS bug

[Nikolas Zimmermann]

Good morning khtml crew,

while integrating svg css properties in khtml, I found a pretty
serious css bug affecting kdelibs trunk & 3.5 branch (as found
out by Charles Samuels, thanks for that).

The full story:
DocumentImpl::attach() causes a CSSStyleSelector to be created, whose
constructor calls CSSStyleSelector::init(). If the 's_defaultStyle' object
isn't available it'll be created through CSSStyleSelector::loadDefaultStyle().
s_defaultSheet & s_quirksSheet (& s_svgSheet on my hdd) are created
with a null CSSStyleSheetImpl ptr. That means the resulting sheets will
have m_doc = 0.

Now the default stylesheet loading proceeds, and as soon as we hit
html4.css which contains @namespace "http://www.w3.org/1999/xhtml";
the problem occours:

css/parser.cpp (generated by parser.y) executes this stuff upon @namespace:

     CSSParser *p = static_cast<CSSParser *>(parser);
    if (p->styleElement && p->styleElement->isCSSStyleSheet())
        static_cast<CSSStyleSheetImpl*>(p->styleElement)->addNamespace(p, 
domString($3), domString($4));

Please see the addNamespace function below (with added dbg statements).

void CSSStyleSheetImpl::addNamespace(CSSParser* p, const DOM::DOMString&
prefix, const DOM::DOMString& uri)
{
    int exceptioncode = 0;
    if (uri.isEmpty())
        return;

    kDebug() << " prefix: " << prefix << " uri: " << uri << " m_namespaces: "
                  << m_namespaces << " m_doc: " << m_doc << endl;

    m_namespaces = new CSSNamespace(prefix, uri, m_namespaces);

    if (prefix.isEmpty())
        p->defaultNamespace = m_doc->getId(NodeImpl::NamespaceId, 
uri.implementation(), false, false, &exceptioncode);
}

After adding this kDebug() statement to the addNamespace() function,
you'll notice following output: 

khtml (css): @namespace: http://www.w3.org/1999/xhtml
testkhtml: prefix:  uri: http://www.w3.org/1999/xhtml m_namespaces: (nil) 
m_doc: (nil)

prefix.isEmpty() returns true, and m_doc is nil.
Somehow it doesn't crash and "works". No idea why.

I noticed this behaviour after I added s_svgSheet + loading code
to CSSStyleSelector::loadDefaultStyle -> it crashed, because of m_doc == 0.

Adding a Q_ASSERT(m_doc != 0) in the if(prefix.isEmpty()) branch
will have the result, that you can not load any files anymore :-)

Even more scary is that it's in since quite a long time:
<quote>
Modified Fri May 20 15:44:37 2005 UTC (10 months ago) by carewolf 
Merge/port/implement XHTML namespace selectors,
and a few other XHTML improvements.
</quote>

WebKit handles this completely different, so I tried to come up with a
solution for our current codebase, included in the attached patch.
(It is created against kdelibs trunk, but also applies against 3.5 branch)

I assured that the created default style sheets got a valid m_doc param.
The newly added asserts (Q_ASSERT(m_doc != 0);) aren't hit anymore
(at least not while running testregression).

No new regressions are introduced, my svg sheet loading works as
expected now, though as this is a hot code path I'd like to have
more input & opinions on the patch.

Bye
 Bye
  Niko

P.S. If I overlooked something stupid, I have an excuse: loooong working day!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: khtml-possible-css-fix.diff
Type: text/x-diff
Size: 4133 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20060321/9f8e7289/attachment.diff>