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

LibWeb: Implement Location.ancestorOrigins #623

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jamierocks
Copy link
Contributor

No description provided.

@@ -404,6 +429,22 @@ WebIDL::ExceptionOr<void> Location::assign(String const& url)
return {};
}

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#dom-location-ancestororigins
WebIDL::ExceptionOr<JS::GCPtr<DOMStringList>> Location::ancestor_origins() const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at this a couple weeks ago, and came to the conclusion that it was a bit of an awkward API to implement correctly. As the spec mentions, there's a lot of discussion on whatwg/html#1918. It seems as though this API was added by Blink initially, for the express(?) purpose of enabling advertisers to ensure that their ads are only being embedded on the correct pages. Other engines are cagey about implementing it. Even so, browsers like Brave take extra steps to prune the list of ancestor origins to make sure that there's less privacy or tracking concerns.

So, my take on this is, if we do implement this API, we should put a big ol FIXME in there and open up an issue to discuss how to "prune" the entries of this list to get the privacy properties we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I did notice that Firefox doesn't implement this API - though curiously (to me), Safari does.

Perhaps we should follow Firefox then, and omit implementing this API - I'm always up for taking the privacy-cautious approach. We wouldn't be alone in skipping this part of the standard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether we implement it as specced, implement it in a constrained way, or choose not to implement it, an issue to point to so we can say we've looked at it and aren't sure what to do will probably be worthwhile. I've asked about it on the whatwg matrix channel, waiting to see what the feedback is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems their recommendation is to ask to bring it up at the next WHATNOT meeting this upcoming Thursday. If you have other work that depends on the DOMStringList change, might be a good idea to pull that out into another PR :)

@ADKaster
Copy link
Member

Per the notes on whatwg/html#10471, WebKit is okay with changing the ancestorOrigins API to address BZ's concerns. An action item was taken to find an applicable Chromium person to give a yay/nay. After that the group will look to find a champion to fix up the spec accordingly.

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

Successfully merging this pull request may close these issues.

2 participants