[Webkit-unassigned] [Bug 12579] WebKit fails SVG xml:base test

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=12579





------- Comment #3 from rwlbuis at gmail.com  2007-03-05 13:11 PDT -------
Hi Darin,

(In reply to comment #2)
> (From update of attachment 13474 [edit])
> The Node class is one of the ones that has some automatically generated
> bindings along with some manually-done legacy ones. Because of this, you should
> put the baseURL property into Node.idl instead of making the changes to
> kjs_dom.cpp and kjs_dom.h so you don't create more legacy for us to deal with
> later.

I think only the Node constructor is generated, the properties not AFAICS?

> To me it looks like baseURI should be a virtual function. It's a little strange
> to use a case statement instead. Or at least there should be an inner part
> that's a virtual function.

Possible.

> +            xmlbase = KURL(static_cast<const
> Element*>(this)->getAttribute(XMLNames::baseAttr).deprecatedString());
> +
> +            if (!xmlbase.protocol().isEmpty())
> +                return xmlbase.url();
> 
> The above code seems like an inefficient way to do the job. Basically we're
> just looking for a ":" but we convert both to and from a DeprecatedString. This
> really cries out for some simple URL-related functions we can just call on
> strings. But I guess it's no big deal.

Possible. I need to check the kdom method KURL::isRelative.

> Why isn't baseURI using the Document::completeURL function? It seems to be
> replicating what it does.

I am not seeing how. The baseURI algorithm may need multiple parent iterations,
while completeURL just uses the current node. It is described here:

http://www.w3.org/TR/2001/REC-xmlbase-20010627/

> Same comment for SVGImageLoader::updateFromElement. Why isn't it using
> Document::completeURL? If it's really different than the standard relative URL
> rules, then I'd like to see test cases demonstrating that.

I hope I am right, but I honestly see how :)

> Is the rule about resolving URLs really different for SVG that HTML? If so,
> then can we put a new function on Document instead of giving SVGImageLoader its
> own relative-URL completion code?

Well it seems different due to xml:base usage at least.

> -            if (attr->name().matches(XLinkNames::hrefAttr))
> +            if (attr->name().matches(XLinkNames::hrefAttr) &&
> !ownerDocument()->parsing())
> 
> Are you sure parsing is the right check here? Maybe a better check is just to
> see if you're attached, since you are doing the update in attach().

That seems a bit better, I did that locally now.

> I think that baseURI and documentURI are not really needed to fix this. There's
> no reason SVGImageLoader has to use new functions from the DOM standard just to
> resolve a relative URL, is there?

I think xmlbase usage is a bit more complex than completeURL. Please let me
know if I got that wrong though, as it is pretty fundamental for the patch :)
I'll probably attach a new patch soon, since the attached and virtual
suggestions sound great to me.
Cheers,

Rob.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.