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

Make the Location object deal when it does not have a browsing context #4076

Merged
merged 3 commits into from
Oct 31, 2018
Merged

Make the Location object deal when it does not have a browsing context #4076

merged 3 commits into from
Oct 31, 2018

Conversation

annevk
Copy link
Member

@annevk annevk commented Oct 9, 2018 •
edited by pr-preview bot
Loading

@annevk annevk mentioned this pull request Oct 18, 2018
Closed
domenic
domenic requested changes Oct 22, 2018
View reviewed changes
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Let's fix the foundational definition, but otherwise the changes seem good.

source Outdated
data-x="concept-document-bc">browsing context</span>'s <span>active document</span>.</p>
data-x="concept-document-bc">browsing context</span>'s <span>active document</span>, if this
<code>Location</code> object's associated <code>Document</code> object's <span
data-x="concept-document-bc">browsing context</span> is a <span>browsing context</span>, and null
Copy link
Member

Choose a reason for hiding this comment

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

At the moment we say "has a browsing context", we don't check whether the slot is a BC or null.

Also what is a Location object's associated Document? Maybe we should go through relevant global object instead?

The relevant Document of a Location object loc is loc's relevant settings object's responsible document, if that Document is active, and null otherwise.

I think this is equivalent??

Copy link
Member

Choose a reason for hiding this comment

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

As discussed on GitHub, an alternative is

The relevant Document of a Location object loc is loc's relevant global object's browsing context's active document.

However both definitions need to a note to point to #1073 to clarify that removed iframe documents are non-active / the "active document" of a browsing context of a removed iframe is null.

source Outdated
@@ -81560,6 +81591,11 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
user agent must run the appropriate steps from the following list:</p>

<dl class="switch">
Copy link
Member

Choose a reason for hiding this comment

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

It's weird that this is a dl instead of an ol. (Preexisting problem.)

source Outdated
@@ -81615,6 +81651,9 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
getter must run these steps:

<ol>
<li><p>If this <code>Location</code> object's <span>relevant <code>Document</code></span> is
null, then return « ».</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

Personally I prefer "an empty list" or "the empty list"; a bit less opaque.

@annevk
Copy link
Member Author

annevk commented Oct 30, 2018

I'm not sure that a pointer to #1073 is needed as removal would also lead to there not being a browsing context so the active document bit doesn't even come into play with the new definition.

@annevk annevk force-pushed the annevk/location-bc-less branch from 394e176 to 953b8cb Compare October 30, 2018 10:38
@annevk
Copy link
Member Author

annevk commented Oct 30, 2018

Implementation bugs:

@cdumez
Copy link

cdumez commented Oct 31, 2018

I support this change and will work on aligning WebKit once this lands.

@domenic domenic merged commit 8ef0102 into master Oct 31, 2018
@domenic domenic deleted the annevk/location-bc-less branch October 31, 2018 19:18
domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 31, 2018
@annevk @domenic
Test Location sans browsing context
fce65eb 
For whatwg/html#3959 and whatwg/html#4076.
kisg pushed a commit to paul99/webkit-mips that referenced this pull request Nov 1, 2018
Location object sans browsing context
4d9bf00 
https://bugs.webkit.org/show_bug.cgi?id=191060

Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

* web-platform-tests/html/browsers/history/the-location-interface/no-browsing-context.window-expected.txt:
Rebase WPT test now that all checks are passing.

* web-platform-tests/html/browsers/history/the-location-interface/no-browsing-context.window.js:
Fix bug in WPT test (web-platform-tests/wpt#13854)

* web-platform-tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window-expected.txt:

Source/WebCore:

As per whatwg/html#4076, a Location object's URL should be "about:blank" when
it does not have a browsing context (Frame), not "".

No new tests, rebaselined existing tests.

* page/Location.cpp:
(WebCore::Location::url const):
(WebCore::Location::href const):
(WebCore::Location::protocol const):
(WebCore::Location::host const):
(WebCore::Location::hostname const):
(WebCore::Location::port const):
(WebCore::Location::pathname const):
(WebCore::Location::search const):
(WebCore::Location::origin const):
(WebCore::Location::hash const):

LayoutTests:

Update existing tests to reflect behavior change.

* http/tests/dom/same-origin-detached-window-properties-expected.txt:
* http/tests/dom/same-origin-detached-window-properties.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window2-pson.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window2.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window3.html:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@237702 268f45cc-cd09-0410-ab3c-d52691b4dbfc
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 10, 2018
@annevk @moz-wptsync-bot
Bug 1497567 [wpt PR 13435] - Test Location sans browsing context, a=t…
d77064c 
…estonly

Automatic update from web-platform-testsTest Location sans browsing context

For whatwg/html#3959 and whatwg/html#4076.
--

wpt-commits: fce65ebd7656136f5f8452378146f1b7af30bf79
wpt-pr: 13435
jyc pushed a commit to jyc/gecko that referenced this pull request Nov 11, 2018
@annevk @moz-wptsync-bot
Bug 1497567 [wpt PR 13435] - Test Location sans browsing context, a=t…
ec1e8e5 
…estonly

Automatic update from web-platform-testsTest Location sans browsing context

For whatwg/html#3959 and whatwg/html#4076.
--

wpt-commits: fce65ebd7656136f5f8452378146f1b7af30bf79
wpt-pr: 13435
@domenic domenic mentioned this pull request Feb 14, 2019
Merged
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
@marco-c
Bug 1497567 [wpt PR 13435] - Test Location sans browsing context, a=t…
a7d3121 
…estonly

Automatic update from web-platform-testsTest Location sans browsing context

For whatwg/html#3959 and whatwg/html#4076.
--

wpt-commits: fce65ebd7656136f5f8452378146f1b7af30bf79
wpt-pr: 13435

UltraBlame original commit: 21440047222db956d995cb518b57b2b3272126e0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
@marco-c
Bug 1497567 [wpt PR 13435] - Test Location sans browsing context, a=t…
e6eba8c 
…estonly

Automatic update from web-platform-testsTest Location sans browsing context

For whatwg/html#3959 and whatwg/html#4076.
--

wpt-commits: fce65ebd7656136f5f8452378146f1b7af30bf79
wpt-pr: 13435

UltraBlame original commit: 21440047222db956d995cb518b57b2b3272126e0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
@marco-c
Bug 1497567 [wpt PR 13435] - Test Location sans browsing context, a=t…
5708552 
…estonly

Automatic update from web-platform-testsTest Location sans browsing context

For whatwg/html#3959 and whatwg/html#4076.
--

wpt-commits: fce65ebd7656136f5f8452378146f1b7af30bf79
wpt-pr: 13435

UltraBlame original commit: 21440047222db956d995cb518b57b2b3272126e0
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request Dec 22, 2020
@cdumez
Location object sans browsing context
177bf52 
https://bugs.webkit.org/show_bug.cgi?id=191060

Reviewed by Geoffrey Garen.

LayoutTests/imported/w3c:

* web-platform-tests/html/browsers/history/the-location-interface/no-browsing-context.window-expected.txt:
Rebase WPT test now that all checks are passing.

* web-platform-tests/html/browsers/history/the-location-interface/no-browsing-context.window.js:
Fix bug in WPT test (web-platform-tests/wpt#13854)

* web-platform-tests/html/webappapis/dynamic-markup-insertion/opening-the-input-stream/url.window-expected.txt:

Source/WebCore:

As per whatwg/html#4076, a Location object's URL should be "about:blank" when
it does not have a browsing context (Frame), not "".

No new tests, rebaselined existing tests.

* page/Location.cpp:
(WebCore::Location::url const):
(WebCore::Location::href const):
(WebCore::Location::protocol const):
(WebCore::Location::host const):
(WebCore::Location::hostname const):
(WebCore::Location::port const):
(WebCore::Location::pathname const):
(WebCore::Location::search const):
(WebCore::Location::origin const):
(WebCore::Location::hash const):

LayoutTests:

Update existing tests to reflect behavior change.

* http/tests/dom/same-origin-detached-window-properties-expected.txt:
* http/tests/dom/same-origin-detached-window-properties.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window2-pson.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window2.html:
* http/tests/security/xss-DENIED-script-inject-into-inactive-window3.html:


Canonical link: https://commits.webkit.org/205976@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237702 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domenic domenic domenic approved these changes

Assignees
No one assigned
Labels
topic: frames/navigables/browsing contexts
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@annevk @cdumez @domenic