[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 #21 from Darin Adler <darin at apple.com> ---
(In reply to Matt Woodrow from comment #20)
> My understanding is that CSSCursorImageValue just wraps a CSSImageValue in
> that case (via m_imageValue), so fixing the completeURL lookups within
> CSSImageValue should be sufficient.

Good point. That seems right!

Do we have test coverage for that case in WPT, or is it just code inspection that gives us the confidence that we handle the cursor image case correctly?

> The call to document.completeURL from within CSSCursorImageValue looks to be
> specific to the case where the cursor property references an element within
> SVG, and we're looking up the href attribute from that.

Agreed, yes.

> > 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.
> 
> The part I'm struggling with is that CSSImageValue wants to use the
> algorithm in Document, whereas the consumers in the parser want to use the
> CSSParserContext algorithm. These differ a fair bit in their resolution of
> the base url, and character encoding.

Well, you should be aware that the one in CSSImageValue is something kind of new, and only used rarely in cases where no base URL is around at the time the CSS value was parsed. I would be surprised if any real world test case relies on these subtle differences between the two, it's just that it needs to happen at a time where we don't have a CSS parsing context. I think we'd probably want it to use the CSS completion algorithm, not the general purpose document one. For now, I would do that by making a cover that takes a document.

> A single completeCSSImageURL function would need to take either a
> CSSParserContext or Document, and pick which completeURL function to call
> based on which was provided at runtime (or I could template it, since the
> args as the same).

Two separate functions would be fine.

I would likely come look at this later and refactor to try to make this have less repeated code, so what matters most is the test coverage, even more than exactly how we right the code.

-- 
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/d99fca38/attachment-0001.htm>