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.

Sign up for GitHub

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

Jump to bottom

LibWeb: Implement Location.ancestorOrigins #623

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

LibWeb: Implement Location.ancestorOrigins #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Userland/Libraries/LibWeb/DOM/Document.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5352,4 +5352,15 @@ void Document::set_cached_navigable(JS::GCPtr<HTML::Navigable> navigable)
m_cached_navigable = navigable.ptr();
}

// https://html.spec.whatwg.org/multipage/document-sequences.html#doc-container-document
JS::GCPtr<DOM::Document> Document::container_document() const
{
// 1. If document's node navigable is null, then return null.
if (!navigable())
return nullptr;

// 2. Return document's node navigable's container document.
return navigable()->container_document();
}

}
3 changes: 3 additions & 0 deletions Userland/Libraries/LibWeb/DOM/Document.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,9 @@ class Document
JS::GCPtr<HTML::Navigable> cached_navigable();
void set_cached_navigable(JS::GCPtr<HTML::Navigable>);

// https://html.spec.whatwg.org/multipage/document-sequences.html#doc-container-document
JS::GCPtr<DOM::Document> container_document() const;

protected:
virtual void initialize(JS::Realm&) override;
virtual void visit_edges(Cell::Visitor&) override;
Expand Down
41 changes: 41 additions & 0 deletions Userland/Libraries/LibWeb/HTML/Location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <LibWeb/Bindings/LocationPrototype.h>
#include <LibWeb/DOM/Document.h>
#include <LibWeb/HTML/CrossOrigin/AbstractOperations.h>
#include <LibWeb/HTML/DOMStringList.h>
#include <LibWeb/HTML/Location.h>
#include <LibWeb/HTML/Navigation.h>
#include <LibWeb/HTML/Window.h>
Expand All @@ -36,6 +37,7 @@ void Location::visit_edges(Cell::Visitor& visitor)
{
Base::visit_edges(visitor);
visitor.visit(m_default_properties);
visitor.visit(m_ancestor_origins_list);
}

void Location::initialize(JS::Realm& realm)
Expand All @@ -48,6 +50,29 @@ void Location::initialize(JS::Realm& realm)
// 5. Set the value of the [[DefaultProperties]] internal slot of location to location.[[OwnPropertyKeys]]().
// NOTE: In LibWeb this happens before the ESO is set up, so we must avoid location's custom [[OwnPropertyKeys]].
m_default_properties.extend(MUST(Object::internal_own_property_keys()));

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#concept-location-ancestor-origins-list
// A Location object has an associated ancestor origins list. When a Location object is created, its ancestor origins
// list must be set to a DOMStringList object whose associated list is the list of strings that the following steps
// would produce:

// 1. Let output be a new list of strings.
Vector<String> output;

// 2. Let current be the Location object's relevant Document.
auto current = relevant_document();

// 3. While current's container document is non-null:
while (current->container_document()) {
// 1. Set current to current's container document.
current = current->container_document();

// 2. Append the serialization of current's origin to output.
output.append(MUST(String::from_byte_string(current->origin().serialize())));
}

// 4. Return output.
m_ancestor_origins_list = DOMStringList::create(realm, move(output));
}

// https://html.spec.whatwg.org/multipage/history.html#relevant-document
Expand Down Expand Up @@ -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 :)

{
// 1. If this's relevant Document is null, then return an empty list.
auto const relevant_document = this->relevant_document();
if (!relevant_document)
return DOMStringList::create(realm(), {});

// 2. If this's relevant Document's origin is not same origin-domain with the entry settings object's origin, then throw a "SecurityError" DOMException.
if (!relevant_document->origin().is_same_origin_domain(entry_settings_object().origin()))
return WebIDL::SecurityError::create(realm(), "Location's relevant document is not same origin-domain with the entry settings object's origin"_fly_string);

// 3. Otherwise, return this's ancestor origins list.
return m_ancestor_origins_list;
}

// 7.10.5.1 [[GetPrototypeOf]] ( ), https://html.spec.whatwg.org/multipage/history.html#location-getprototypeof
JS::ThrowCompletionOr<JS::Object*> Location::internal_get_prototype_of() const
{
Expand Down
5 changes: 5 additions & 0 deletions Userland/Libraries/LibWeb/HTML/Location.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class Location final : public Bindings::PlatformObject {
void reload() const;
WebIDL::ExceptionOr<void> assign(String const& url);

WebIDL::ExceptionOr<JS::GCPtr<DOMStringList>> ancestor_origins() const;

virtual JS::ThrowCompletionOr<JS::Object*> internal_get_prototype_of() const override;
virtual JS::ThrowCompletionOr<bool> internal_set_prototype_of(Object* prototype) override;
virtual JS::ThrowCompletionOr<bool> internal_is_extensible() const override;
Expand Down Expand Up @@ -83,6 +85,9 @@ class Location final : public Bindings::PlatformObject {

// [[DefaultProperties]], https://html.spec.whatwg.org/multipage/history.html#defaultproperties
Vector<JS::Value> m_default_properties;

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#concept-location-ancestor-origins-list
JS::GCPtr<DOMStringList> m_ancestor_origins_list;
};

}
4 changes: 3 additions & 1 deletion Userland/Libraries/LibWeb/HTML/Location.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#import <HTML/DOMStringList.idl>

// https://html.spec.whatwg.org/multipage/nav-history-apis.html#location
[Exposed=Window]
interface Location { // but see also additional creation steps and overridden internal methods
Expand All @@ -15,5 +17,5 @@ interface Location { // but see also additional creation steps and overridden in
[LegacyUnforgeable] undefined replace(USVString url);
[LegacyUnforgeable] undefined reload();

[FIXME, LegacyUnforgeable, SameObject] readonly attribute DOMStringList ancestorOrigins;
[LegacyUnforgeable, SameObject] readonly attribute DOMStringList ancestorOrigins;
};