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

Extending window.location #5523

Closed
bokand opened this issue May 8, 2020 · 5 comments
Closed

Extending window.location #5523

bokand opened this issue May 8, 2020 · 5 comments

Comments

@bokand
Copy link
Contributor

bokand commented May 8, 2020

Filing this to get broader feedback on how to treat new APIs on window.location.

When Blink shipped TextFragments it added a new attribute: window.location.fragmentDirective. This is currently used only for feature detection but planned to potentially allow new abilities like reading the fragment directive and invoking it post-load.

At first glance, window.location seemed like a reasonable place for it since it's analogous to window.locaiton.hash. However, as crbug/1057795 shows, window.location has some legacy quirks. In this case, we changed the prototype of the location object because we didn't apply the [Unforgeable] attribute to it.

@yuki3 and @domenic argued persuasively we should move fragmentDirective elsewhere. Though it's shipped we expect usage to be sufficiently low that we could potentially do this (we're confirming usage numbers now). However, we wanted to know what others think of this? Should new APIs be discouraged from adding window.location (in which case we could make a note of it in the spec)? Or should we work out the kinks so that APIs that truly belong could be added?

@annevk
Copy link
Member

annevk commented May 9, 2020

This was not discussed in #4868.

You probably want to read https://bugzilla.mozilla.org/show_bug.cgi?id=1082734. Tear-off objects on Location should not be added I think.

New properties that do not return an associated object seem fine and I tend to think we should continue to use [LegacyUnforgeable] for those for consistency.

@bokand
Copy link
Contributor Author

bokand commented May 21, 2020

Sorry if this is a dumb question but what are "tear off" and "associated" objects?

Right now fragmentDirective is used only for feature detection so link generators like search engines can avoid creating fragment directives unless the UA supports them. However, this seems like a natural extension point and there's a few use cases we've considered that might eventually warrant additional API surface (e.g. the Marginalia example in mozilla/standards-positions#194).

It definitely seems problematic in our case that pages could save a reference to it, navigate, and read the new values. Given that document loading is where the fragment is read, stripped, and stored I think Document is probably a better place for it but it sounds like there might still be valid cases to add new things to window.location.

@domenic
Copy link
Member

domenic commented May 21, 2020

Those refer to any non-primitive value (i.e., object value) which is associated with the Location object, and accessible via a property on it. I think the proposed fragmentDirective is one of those, right? I.e. it's an empty object, not a string?

@bokand
Copy link
Contributor Author

bokand commented May 21, 2020

Yeah, it's an empty object (and could potentially become non-empty in the future).

@bokand
Copy link
Contributor Author

bokand commented Aug 19, 2020

FYI: we've moved the fragmentDirective to document. Unless there's any kind of note or warning y'all think might be worth adding to HTML I'm happy to close this issue.

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

No branches or pull requests

3 participants