ladybird icon indicating copy to clipboard operation
ladybird copied to clipboard

LibWeb: Support clickable `<a>` tags in SVGs

Open dosisod opened this issue 5 months ago • 3 comments

Summary

Currently <a> tags in SVGs are not clickable, and do not navigate to other pages.

Operating system

Linux

Steps to reproduce

  1. Go to https://svgwg.org/svg2-draft/images/linking/link01.svg
  2. Click the red ellipse

Expected behavior

It should navigate to https://www.w3.org/

Actual behavior

It does nothing.

URL for a reduced test case

https://svgwg.org/svg2-draft/images/linking/link01.svg

HTML/SVG/etc. source for a reduced test case

N/A

Log output and (if possible) backtrace

None

Screenshots or screen recordings

Before clicking red ellipse:

Image

After clicking:

Image

Build flags or config settings

No response

Contribute a patch?

  • [x] I’ll contribute a patch for this myself.

dosisod avatar Nov 10 '25 18:11 dosisod

I'm currently working on a PR for this. The code is functional, but requires some hacks due to IDL conflicts.

The issue is that the IDL generator doesn't support the ReflectSetter extended attribute. This is currently causing a conflict because the href attributes are specified in multiple places:

SVGAElement.idl

[Exposed=Window]
interface SVGAElement : SVGGraphicsElement {
  // ...
};

SVGAElement includes SVGURIReference;
SVGAElement includes HTMLHyperlinkElementUtils;

SVGURIReference.idl

interface mixin SVGURIReference {
  [SameObject] readonly attribute SVGAnimatedString href;  // <----- href specified here
};

HTMLHyperlinkElementUtils

interface mixin HTMLHyperlinkElementUtils {
  [CEReactions, ReflectSetter] stringifier attribute USVString href;  // <---- and here
};

This seems to be filed at #5631 and partially implemented in #2875, although that PR did not implement ReflectSetter.

dosisod avatar Dec 11 '25 00:12 dosisod

I found an existing spec issue about the conflicting href definitions: https://github.com/w3c/svgwg/issues/312 - there's some (relatively) recent discussion about it. Specifically:

Should SVGAElement just stop including HTMLHyperlinkElementUtils? It's not implemented in any of the 4 engines I checked.

That sounds like a reasonable solution for now.

As for the attribute, it's fine to add a FIXME to the .idl file to say that it should have the attribute, but we don't support it yet. I wouldn't recommend trying to modify the IDL generator as a first contribution. 😅

AtkinsSJ avatar Dec 11 '25 09:12 AtkinsSJ

I peeked at the IDL generator and quickly realized I was not going to slay that dragon. 😅

Since I didn't want to re-implement the link following logic in HTMLHyperlinkElementUtils for SVGs, I created a SVGHyperlinkElementUtils class which extends the HTML one, then lazy load it on click:

void SVGAElement::activation_behavior(Web::DOM::Event const& event) {
    // ...

    if (m_hyperlink_utils == nullptr)
        m_hyperlink_utils = adopt_own(*new SVGHyperlinkElementUtils(*this, document()));

    m_hyperlink_utils->follow_the_hyperlink(hyperlink_suffix, user_involvement);
}

This probably isn't ideal, but it avoids having to rewrite HTMLHyperlinkElementUtils to support the SVGAnimatedString version of href.

You can poke around my changes here: https://github.com/LadybirdBrowser/ladybird/compare/master...dosisod:ladybird:clickable-svgs . I have to clean up a few things before I open a PR, but once I mimic the IDL interface for SVGAElement it should be ready for review!

dosisod avatar Dec 11 '25 22:12 dosisod