[Webkit-unassigned] [Bug 229917] Correctly support fragment-only URLs in CSS images

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=229917

--- Comment #19 from Darin Adler <darin at apple.com> ---
Comment on attachment 444322
  --> https://bugs.webkit.org/attachment.cgi?id=444322
Patch

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

>>> Source/WebCore/dom/Document.cpp:5606
>>> +        return URL(URL(), url);
>> 
>> I think you are correct to say that it may be incorrect to have this here. We’ll need to see which regression tests fail.
>> 
>> Instead we may want to change CSSImageValue to use another function specific to CSS that adds this behavior and then calls through to Document::completeURL. I might put it in CSSParserContext as a static member function that takes a document, or somewhere else that CSSImageValue can find it. I think there are possibly other call sites like CSSCursorImageValue that may need the same code.
> 
> I'm not sure about CSSCursorImageValue, it looks like that's checking the 'href' attribute of an SVG cursor element and likely isn't covered by the css values spec.
> 
> How about a static 'should this string be kept as-is without resolving relative to the base url' static helper to CSSValue, and then have both the parser and CSSImageValue check that?

CSSCursorImageValue has URLs that come from the CSS "cursor" property. Syntax like: "cursor: url(hand.cur), pointer" that is parsed by the consumeCursor function in CSSPropertyParser.cpp. It’s simply got less test coverage than CSSImageValue, but it has many of the same issues.

It’s fine to put a static helper somewhere; it’s just much better to have this packaged as part of a URL completion algorithm rather than a check outside. Having a separate check will result in repetitive code outside the function. So maybe we should just add new completeImageURL or completeCSSImageURL functions that wrap the existing completeURL functions.

And yes, still nice to share the check for which URLs should not be resolved relative to the base for this case, and name based on the specification. Like a function named isCSSLocalURL.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211116/43fcc254/attachment.htm>