KURL(urlString) feels like something one should use by default, but it really isn't. This form can only be used with strings that are normalized in KURL sense - most notably, with strings that are returned by KURL::string() and stored as String objects to reduce memory usage as compared to KURL objects.
Created attachment 38830 [details] proposed patch
Comment on attachment 38830 [details] proposed patch Maybe the name of this should tie in with the name of the function that lets you get the string out of the KURL in the first place. I wish there was a comment mentioning that it's usually a design mistake to be re-creating the KURL from an already parsed string unless we want to save storage by storing a string instead of a KURL. Is that true? If so, where should we say it. > - KURL completedURL = url.isEmpty() ? KURL("") : completeURL(exec, url); > + KURL completedURL = url.isEmpty() ? KURL(ParsedURLString, "") : completeURL(exec, url); I think a function named emptyURL() in KURL.h would be even better for cases like this. > - KURL base(getAttribute(baseAttr)); > + KURL base(ParsedURLString, getAttribute(baseAttr)); Can this possibly be right? Can't the attribute be set to anything? What guarantees that it's a parsed URL string? r=me on the mechanical change, clearly correct, but I think we still have a ways to go to make this clear
> I wish there was a comment mentioning that it's usually a design mistake to be > re-creating the KURL from an already parsed string unless we want to save > storage by storing a string instead of a KURL. Is that true? If so, where > should we say it. I think I said something similar in ChangeLog, but having a comment in code seems fine. I'm going to add the following before the constructor declaration: "It is usually best to avoid repeatedly parsing a string, unless memory saving outweigh the possible slow-downs." > > - KURL base(getAttribute(baseAttr)); > > + KURL base(ParsedURLString, getAttribute(baseAttr)); > > Can this possibly be right? Can't the attribute be set to anything? What > guarantees that it's a parsed URL string? Yes, this is wrong. Looks like document base should serve as base for this one. I'll file a bug. This only affects xml:base. > r=me on the mechanical change, clearly correct, but I think we still have a > ways to go to make this clear I think that the main shortcoming here is that this is not enforced by the compiler in any way.
Filed bug 28858 for xml:base and baseURI.
Committed revision 47907.