Bug 113630 – HTMLLinkElement should resolve resource URLs when resources will be fetched

• Component: DOM
• Status: RESOLVED FIXED

[Rafael Weinstein]

Currently it resolves urls when the href attribute is processed (either during parse or setAttribute).

This will result in an erroneous url if 

a) the document's base URL has changed since the href attribute was processed
b) the link was adopted from another document

[Rafael Weinstein]

Created attachment 195823 [details]
Patch

[Rafael Weinstein]

Created attachment 195825 [details]
Patch

[Rafael Weinstein]

this time with layout test attached.

[Eric Seidel (no email)]

Comment on attachment 195825 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195825&action=review

> Source/WebCore/ChangeLog:6
> +         HTMLLinkElement was resolving its URL when the href attribute was processed and caching it without ever

Extra space.

> Source/WebCore/html/HTMLLinkElement.cpp:138
> +    String url = stripLeadingAndTrailingHTMLSpaces(href);
> +    return url.isEmpty() ? KURL() : document()->completeURL(url);

I'm surprised we need this ternary?  Since I would expect other palces to treat empty urls as urls to the document?  I guess this si so the later isValid() check works?

> Source/WebCore/html/HTMLLinkElement.h:108
> +    KURL getURL() const;

I believe webkit style is to avoid using "get" in getters.

[Rafael Weinstein]

Created attachment 195865 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 195865 [details]
Patch

I believe WebKIt style would have you rename "URL()" to "url()".  Otherwise this LGTM.

[Rafael Weinstein]

Created attachment 195867 [details]
Patch

[Rafael Weinstein]

Created attachment 195868 [details]
Patch for landing

[Rafael Weinstein]

just noticed getNonEmptyURLAttribute() on Element.cpp. using that instead.

[Adam Barth]

Comment on attachment 195868 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=195868&action=review

> Source/WebCore/html/HTMLLinkElement.cpp:180
> +    KURL url =  getNonEmptyURLAttribute(hrefAttr);

You've got an extra space after the = on this line.

[Rafael Weinstein]

Created attachment 195878 [details]
Patch for landing

[Rafael Weinstein]

Comment on attachment 195868 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=195868&action=review

>> Source/WebCore/html/HTMLLinkElement.cpp:180
>> +    KURL url =  getNonEmptyURLAttribute(hrefAttr);
> 
> You've got an extra space after the = on this line.

fixed.

[WebKit Review Bot]

Comment on attachment 195878 [details]
Patch for landing

Clearing flags on attachment: 195878

Committed r147291: <http://trac.webkit.org/changeset/147291>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.