linkedom icon indicating copy to clipboard operation
linkedom copied to clipboard

`baseURI` should be factored into `HTMLAnchorElement.href`

Open danburzo opened this issue 2 years ago • 5 comments

Node.baseURI should be used when getting the HTMLAnchorElement.href property. (It maybe needed for other properties as well, will add them here once I investigate.)

danburzo avatar Aug 17 '22 09:08 danburzo

I guess it should also be removed if added as part of the href, right?

WebReflection avatar Aug 17 '22 09:08 WebReflection

At first sight, I would say href should return something like new URL(this.getAttribute('href'), this.baseURI).href, but I'll read the spec closer to see if I'm missing anything.

danburzo avatar Aug 17 '22 09:08 danburzo

One thing I can't figure out is that even though HTMLImageElement.src demonstrably behaves similarly to HTMLAnchorElement.href, I can't find the part of the spec that describes the behavior. ~~This is not the case, so it must be overridden somewhere~~:

The alt, src, srcset and sizes IDL attributes must reflect the respective content attributes of the same name.

Update: The definition for reflecting an attribute says:

If a reflecting IDL attribute has the type USVString:

  • The getter steps are:
    1. If the content attribute is defined to contain a URL:
      1. If the content attribute is absent, then return the empty string.
      2. Parse the value of the content attribute relative to the element's node document.
      3. If that is successful, then return the resulting URL string.
      4. Otherwise, return the value of the content attribute, converted to a USVString.
    2. Otherwise:
      1. Return the value of the content attribute, converted to a USVString.
  • The setter steps are to set the content attribute's value to the given value.

So I guess this applies at least to:

  • HTMLImageElement.src
  • HTMLImageElement.currentSrc
  • HTMLMediaElement.currentSrc

danburzo avatar Aug 17 '22 10:08 danburzo

In general, it seems like for reflecting content attributes we know to be URLs, it would be useful to implement a new attribute helper, something like:

export const urlAttribute = {
  get(element, name) {
    let attr = element.getAttribute(name);
    if (attr) {
      try {
        return new URL(attr, element.baseURI).href;
      } catch (err) {
        return attr;
      }
    }
    return '';
  },
  set(element, name, value) {
    element.setAttribute(name, value);
  }
};

For <a> and <area> attributes, however, it seems more useful to maintain an internal url property as described in the HTMLHyperlinkElementUtils mixin. This should help with other issues as well:

  • https://github.com/WebReflection/linkedom/issues/151

That being said, I see no harm in supporting just HTMLAnchorElement.href initially, with the help of urlAttribute.

danburzo avatar Aug 17 '22 11:08 danburzo

Adding here as to not lose the conclusion in the PR discussion:

Fixing URL-related IDL properties means including an evaluation of the document's baseURI on a hotpath. This evaluation entails a querySelector that may make things too slow for linkedoms goals.

If anyone wants to evaluate the performance story, I consider the PR to be otherwise in a good state.

danburzo avatar Aug 18 '22 09:08 danburzo