Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

baseURI should be factored into HTMLAnchorElement.href #153

Open
danburzo opened this issue Aug 17, 2022 · 5 comments · May be fixed by #154
Open

baseURI should be factored into HTMLAnchorElement.href #153

danburzo opened this issue Aug 17, 2022 · 5 comments · May be fixed by #154

Comments

@danburzo
Copy link

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 danburzo changed the title baseURI should be factored into HTMLAnchorElement.href baseURI should be factored into HTMLAnchorElement.href Aug 17, 2022
@WebReflection
Copy link
Owner

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

@danburzo
Copy link
Author

danburzo commented Aug 17, 2022

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
Copy link
Author

danburzo commented Aug 17, 2022

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
Copy link
Author

danburzo commented Aug 17, 2022

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:

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

@danburzo
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants