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

Change initial about:blank navigation behavior for iframes and popups #6869

Merged
merged 8 commits into from
Aug 10, 2021
Merged

Change initial about:blank navigation behavior for iframes and popups #6869

Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2f751c9
Do not navigate to about:blank on iframe insertion
domenic Jul 16, 2021
bdf3e17
No load events anywhere
domenic Jul 21, 2021
a39ae5e
Fix bug in last commit and refactor window open steps a lot
domenic Jul 21, 2021
af0c760
Review comments
domenic Jul 26, 2021
cd5f327
Do *not* check query and fragment
domenic Jul 27, 2021
e0dc4c9
URL and history update steps
domenic Aug 2, 2021
5bc2d13
Fire the load event sync
domenic Aug 5, 2021
504d117
Expand the note per review
domenic Aug 10, 2021
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
196 changes: 129 additions & 67 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6575,6 +6575,18 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<span>tree order</span>.</p></li>
</ol>

<p>A <span>URL</span> <dfn>matches <code>about:blank</code></dfn> if its <span
data-x="concept-url-scheme">scheme</span> is "<code data-x="">about</code>", its <span
data-x="concept-url-path">path</span> contains a single string "<code data-x="">blank</code>", its
<span data-x="concept-url-username">username</span> and <span
data-x="concept-url-password">password</span> are the empty string, and its <span
data-x="concept-url-host">host</span> is null.</p>

<p class="note">Such a URL's <span data-x="concept-url-query">query</span> and <span
data-x="concept-url-fragment">fragment</span> can be non-null. For example, the <span>URL
record</span> created by <span data-x="URL parser">parsing</span> "<code
data-x="">about:blank?foo#bar</code>" <span>matches <code>about:blank</code></span>.</p>


<div w-nodev>

Expand Down Expand Up @@ -9199,7 +9211,7 @@ o.myself = o;</code></pre>
<span>nested browsing context</span>'s <span data-x="bc-container-document">container
document</span>'s <span data-x="concept-document-origin">origin</span>, even though its
<span>active <code>Document</code></span>'s <span data-x="concept-document-url">URL</span> is
"<code>about:blank</code>". <ref spec=DOM></p>
<code>about:blank</code>. <ref spec=DOM></p>

<p>When a <code>Document</code> is created by a <span data-x="concept-script">script</span> using
the <code data-x="dom-DOMImplementation-createDocument">createDocument()</code> or <code
Expand Down Expand Up @@ -30673,39 +30685,48 @@ interface <dfn interface>HTMLIFrameElement</dfn> : <span>HTMLElement</span> {
</ol>
</li>

<li><p>Otherwise, if <var>element</var> has a <code data-x="attr-iframe-src">src</code> attribute
specified, or <var>initialInsertion</var> is false, then run the <span>shared attribute
processing steps for <code>iframe</code> and <code>frame</code> elements</span> given
<var>element</var>.</p></li>
<li><p>Otherwise, run the <span>shared attribute processing steps for <code>iframe</code> and
<code>frame</code> elements</span> given <var>element</var> and
<var>initialInsertion</var>.</p></li>
</ol>

<p id="otherwise-steps-for-iframe-or-frame-elements">The <dfn>shared attribute processing steps
for <code>iframe</code> and <code>frame</code> elements</dfn>, given an element
<var>element</var>, are:</p>
<var>element</var> and a boolean <var>initialInsertion</var>, are:</p>

<ol>
<li>
<p>If <var>element</var> has no <code data-x="attr-iframe-src">src</code> attribute specified,
or its value is the empty string, let <var>url</var> be the <span>URL</span>
"<code>about:blank</code>".</p>
<!-- https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=2641 -->

<p>Otherwise, <span data-x="parse a url">parse</span> the value of <var>element</var>'s <code
data-x="attr-iframe-src">src</code> attribute, relative to <var>element</var>'s <span>node
document</span>.</p>
<li><p>Let <var>url</var> be the <span>URL record</span> <code>about:blank</code>.</p></li>

<p>If that is not successful, then let <var>url</var> be the <span>URL</span>
"<code>about:blank</code>". Otherwise, let <var>url</var> be the <span>resulting URL
record</span>.</p>
</li>
<li><p>If <var>element</var> has a <code data-x="attr-iframe-src">src</code> attribute specified,
and its value is not the empty string, then <span data-x="parse a url">parse</span> the value of
Copy link
Member

Choose a reason for hiding this comment

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

This is an existing problem I think, but what if it's about:blank?test or about:blank#test?

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent is that it's an exact comparison to the about:blank URL record. However I agree this is not very clear. WDYT of introducing an auxiliary definition (in HTML) for "is the about:blank URL", which does proper component-wise comparisons?

Or I guess we could just compare the serialization to the string "about:blank".

I believe @rakina had tests for this in a different PR but we should probably expand web-platform-tests/wpt#29745 to include them too.

Copy link
Member

Choose a reason for hiding this comment

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

Comparing components seems ideal, but either is okay I think.

Copy link
Member

Choose a reason for hiding this comment

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

I've added about:blank#foo and about:blank?foo to the tests. Everything seems to be OK in Chrome and Safari except for 1 (load event on <iframe> element for about:blank#foo) which is probably a bug.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably also worth checking that the URI of the resulting document is actually updated for about:blank?foo and about:blank#foo, so that location.href, location.search, location.hash, and document.documentURI are correct. If we just stuck with the initial about:blank document & fire a load event in that situation naively, we wouldn't update these parameters.

A probably unrelated issue, but it appears that in Chrome about:blank?foo has location.search == "?foo" but Firefox appears to have location.search == "".

Copy link
Member

@rakina rakina Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Chrome does something so that the resulting iframe is on about:blank?foo.
What is that something? Is it a full navigation from the initial about:blank to another "initial about:blank?foo"?

Ah, I see, sorry for missing the big question! In Chrome, new browsing contexts always start with the initial about:blank document, but then we do the second "initial" navigation synchronously on it when the src is not set/empty/matches about:blank. It is a full navigation, so it would update the URL, and create a new document, etc, but it happens synchronously, and we still treat it as the "initial empty document" after.

So for about:blank it is equivalent to not navigating at all and just firing the load event, like in this PR.
Both about:blank#foo and about:blank?foo also goes through the synchronous navigation path, updating the URL but leaving the "initial empty document status" true.

  • For about:blank#foo I guess it's fine to just make it fire a navigation in the spec too, since it will be a same-document navigation.
  • For about:blank?foo though, hmm, I wonder if it makes more sense to just update the URL from here, or trigger a full navigation but... indicate that it's synchronous and keeps the "initial about:blank status"? Or maybe if that's awkward maybe we can look into changing Chrome's behavior to only happen on just about:blank exactly (and empty/invalid src) [or at least filing a bug for it :)] as I suspect changing the behavior of about:blank?foo is not as risky as changing the behavior of those other cases (not really sure tho)

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, right, this makes sense given your original #6863!

Do you think such a "synchronous navigation" would be the same as running the URL and history update steps (with isPush set to false, i.e. replacement)? I think that would be equivalent to updating both the document's URL and the current session history entry's URL to about:blank?foo or about:blank#foo. (But, using the shared primitive of "URL and history update steps" makes me feel better than doing that directly.)

That might be the easiest way to spec something that matches 2/3 browsers without introducing new concepts.

Copy link
Member

@rakina rakina Jul 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think such a "synchronous navigation" would be the same as running the URL and history update steps (with isPush set to false, i.e. replacement)?

Ooh, right, we already have that. I think that works! (and that gave me inspiration on what to migrate those cases to in the implementation, thanks :D)

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I pushed a change that does the URL and history update steps for the "matches about:blank" branch. I'd love to get another review from @mystor, and @rakina and @annevk should feel free to take another look as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ping @mystor, would you be able to take another look?

that attribute relative to <var>element</var>'s <span>node document</span>. If this is
domenic marked this conversation as resolved.
Show resolved Hide resolved
successful, then set <var>url</var> to the <span>resulting URL record</span>.</p></li>

<li><p>If there exists an <span>ancestor browsing context</span> of <var>element</var>'s
<span>nested browsing context</span> whose <span>active document</span>'s <span
data-x="concept-document-url">URL</span>, ignoring <span
data-x="concept-url-fragment">fragments</span>, is equal to <var>url</var>, then return.</p></li>
<!-- https://www.hixie.ch/tests/adhoc/html/frames/iframes/ 008.html and 009.html -->
domenic marked this conversation as resolved.
Show resolved Hide resolved
<!-- https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=1969 -->
<!-- I'm assuming that "resolve" will normalize things like scheme and hostname case -->

<li>
<p>If <var>url</var> <span>matches <code>about:blank</code></span> and
<var>initialInsertion</var> is true, then:</p>

<ol>
<li>
<p>Perform the <span>URL and history update steps</span> given <var>element</var>'s
<span>nested browsing context</span>'s <span>active document</span> and <var>url</var>.</p>

<p class="note">This is necessary in case <var>url</var> is something like <code
data-x="">about:blank?foo</code>.</p>
domenic marked this conversation as resolved.
Show resolved Hide resolved
</li>

<li><p>Run the <span>iframe load event steps</span> given <var>element</var>.</p></li>

<li><p>Return.</p></li>
</ol>
</li>

<li><p>Let <var>resource</var> be a new <span data-x="concept-request">request</span> whose <span
data-x="concept-request-url">URL</span> is <var>url</var> and whose <span
Expand Down Expand Up @@ -32081,11 +32102,12 @@ interface <dfn interface>HTMLObjectElement</dfn> : <span>HTMLElement</span> {
<p>If the <code>object</code> element's <span>nested browsing context</span> is null, then
<span>create a new nested browsing context</span> for the element.</p>

<p>If the <span>URL</span> of the given resource is not <code>about:blank</code>, then
<span>navigate</span><!-- DONAV object --> the element's <span>nested browsing
context</span> to that resource, with <var data-x="navigation-hh">historyHandling</var> set
to "<code data-x="hh-replace">replace</code>" and the <span>source browsing context</span>
set to the <code>object</code> element's <span>node document</span>'s <span
<p>If the <span>URL</span> of the given resource does not <span data-x="matches
about:blank">match <code>about:blank</code></span>, then <span>navigate</span><!-- DONAV
object --> the element's <span>nested browsing context</span> to that resource, with <var
data-x="navigation-hh">historyHandling</var> set to "<code
data-x="hh-replace">replace</code>" and the <span>source browsing context</span> set to the
<code>object</code> element's <span>node document</span>'s <span
data-x="concept-document-bc">browsing context</span>. (The <code
data-x="attr-object-data">data</code> attribute of the <code>object</code> element doesn't
get updated if the browsing context gets further navigated to other locations.)</p>
Expand Down Expand Up @@ -79200,8 +79222,8 @@ popup4.close();</code></pre></div>
origin</span>.</p></li>

<li>
<p>If <var>invocationOrigin</var> is non-null and <var>url</var> is <code>about:blank</code>,
then return <var>invocationOrigin</var>.</p>
<p>If <var>invocationOrigin</var> is non-null and <var>url</var> <span>matches
<code>about:blank</code></span>, then return <var>invocationOrigin</var>.</p>

<p class="note">The result here is that two documents end up with the same underlying
<span>origin</span>, meaning that <code data-x="dom-document-domain">document.domain</code>
Expand Down Expand Up @@ -80816,69 +80838,92 @@ dictionary <dfn dictionary>WindowPostMessageOptions</dfn> : <span>PostMessageOpt
tab.</p>
</li>

<li><p>Let <var>new</var> be true if <var>windowType</var> is either "<code data-x="">new and
unrestricted</code>" or "<code data-x="">new with no opener</code>", and false otherwise.</p></li>

<li><p>If <var>target browsing context</var> is null, then return null.</p></li>

<li><p>If <var>new</var> is true, then <span>set up browsing context features</span> for
<var>target browsing context</var> given <var>tokenizedFeatures</var>. <ref
spec="CSSOMVIEW"></p></li>

<li><p>Let <var>urlRecord</var> be the <span>URL</span> "<code>about:blank</code>".</p></li>

<li>
<p>If <var>url</var> is not the empty string or <var>new</var> is true, then:
<p>If <var>windowType</var> is either "<code data-x="">new and unrestricted</code>" or "<code
data-x="">new with no opener</code>", then:</p>

<ol>
<li><p><span>Set up browsing context features</span> for <var>target browsing context</var>
given <var>tokenizedFeatures</var>. <ref spec="CSSOMVIEW"></p></li>

<li><p>Let <var>urlRecord</var> be the <span>URL record</span>
<code>about:blank</code>.</p></li>

<li><p>If <var>url</var> is not the empty string, then <span data-x="parse a url">parse</span>
<var>url</var> relative to the <span>entry settings object</span>, and set <var>urlRecord</var>
to the <span>resulting URL record</span>, if any. If the <span>parse a URL</span> algorithm
failed, then throw a <span>"<code>SyntaxError</code>"</span> <code>DOMException</code>.</p></li>

<li><p>Let <var>request</var> be a new <span data-x="concept-request">request</span> whose
<span data-x="concept-request-url">URL</span> is <var>urlRecord</var>.</p></li>

<li><p>If <var>noreferrer</var> is true, then set <var>request</var>'s <span
data-x="concept-request-referrer">referrer</span> to "<code
data-x="">noreferrer</code>".</p></li>
failed, then throw a <span>"<code>SyntaxError</code>"</span>
<code>DOMException</code>.</p></li>

<li><p>Let <var>window</var> be <var>target browsing context</var>'s <span>active
window</span>.</p></li>
<li>
<p>If <var>urlRecord</var> <span>matches <code>about:blank</code></span>, then perform the
<span>URL and history update steps</span> given <var>target browsing context</var>'s
<span>active document</span> and <var>urlRecord</var>.</p>

<li><p>If <var>urlRecord</var> is "<code>about:blank</code>" and <var>new</var> is true, then
<span>queue a global task</span> on the <span>networking task source</span> given
<var>window</var> to <span data-x="concept-event-fire">fire an event</span> named <code
data-x="event-load">load</code> at <var>window</var>, with the <var>legacy target override
flag</var> set.</p>
<p class="note">This is necessary in case <var>urlRecord</var> is something like <code
data-x="">about:blank?foo</code>.</p>
</li>

domenic marked this conversation as resolved.
Show resolved Hide resolved
<li>
<p>Otherwise:</p>

<ol>
<li><p>Let <var>historyHandling</var> be "<code data-x="hh-replace">replace</code>" if
<var>new</var> is true; otherwise "<code data-x="hh-default">default</code>".</p></li>
<li><p>Let <var>request</var> be a new <span data-x="concept-request">request</span> whose
<span data-x="concept-request-url">URL</span> is <var>urlRecord</var>.</p></li>

<li><p>If <var>noreferrer</var> is true, then set <var>request</var>'s <span
data-x="concept-request-referrer">referrer</span> to "<code
data-x="">no-referrer</code>".</p></li>

<li><p><span>Navigate</span><!--DONAV window.open()--> <var>target browsing context</var> to
<var>request</var>, with <var><span>exceptionsEnabled</span></var> set to true, <var
data-x="navigation-hh">historyHandling</var> set to <var>historyHandling</var>, and the
<span>source browsing context</span> set to <var>source browsing context</var>.</p></li>
data-x="navigation-hh">historyHandling</var> set to "<code
domenic marked this conversation as resolved.
Show resolved Hide resolved
data-x="hh-replace">replace</code>", and the <span>source browsing context</span> set to
<var>source browsing context</var>.</p></li>
</ol>
</li>
</ol>
</li>

<li><p>If <var>noopener</var> is true or <var>windowType</var> is "<code
data-x="">new with no opener</code>", then return null.</p></li>

<li>
<p>Otherwise, if <var>new</var> is false, set <var>target browsing context</var>'s <span>opener
browsing context</span> to <var>source browsing context</var>.</p>
<p>Otherwise:</p>

<ol>
<li>
<p>If <var>url</var> is not the empty string, then:</p>

<ol>
<li><p>Let <var>urlRecord</var> be the <span>URL record</span>
<code>about:blank</code>.</p></li>

<p class="note">If <var>new</var> is true this is done as part of <span>creating a new auxiliary
browsing context</span>.</p>
<li><p><span data-x="parse a url">Parse</span> <var>url</var> relative to the <span>entry
settings object</span>, and set <var>urlRecord</var> to the <span>resulting URL
record</span>, if any. If the <span>parse a URL</span> algorithm failed, then throw a
<span>"<code>SyntaxError</code>"</span> <code>DOMException</code>.</p></li>

<li><p>Let <var>request</var> be a new <span data-x="concept-request">request</span> whose
<span data-x="concept-request-url">URL</span> is <var>urlRecord</var>.</p></li>

<li><p>If <var>noreferrer</var> is true, then set <var>request</var>'s <span
data-x="concept-request-referrer">referrer</span> to "<code
data-x="">noreferrer</code>".</p></li>

<li><p><span>Navigate</span><!--DONAV window.open()--> <var>target browsing context</var> to
<var>request</var>, with <var><span>exceptionsEnabled</span></var> set to true and the
<span>source browsing context</span> set to <var>source browsing context</var>.</p></li>
</ol>
</li>

<li><p>If <var>noopener</var> is false, then set <var>target browsing context</var>'s
<span>opener browsing context</span> to <var>source browsing context</var>.</p></li>
</ol>
</li>

<li><p>If <var>noopener</var> is true or <var>windowType</var> is "<code
data-x="">new with no opener</code>", then return null.</p></li>

<li><p>Return <var>target browsing context</var>'s <code>WindowProxy</code> object.</p></li>
</ol>

Expand Down Expand Up @@ -88510,9 +88555,26 @@ dictionary <dfn dictionary>PageTransitionEventInit</dfn> : <span>EventInit</span
<li><p>Set <var>document</var>'s <span>completely loaded time</span> to the current
time.</p></li>

<li><p>Let <var>container</var> be <var>document</var>'s <span
data-x="concept-document-bc">browsing context</span>'s <span
data-x="bc-container">container</span>.</p></li>
<li>
<p>Let <var>container</var> be <var>document</var>'s <span
data-x="concept-document-bc">browsing context</span>'s <span
data-x="bc-container">container</span>.</p>

<div class="note">
<p>This will be null in the case where <var>document</var> is the <span data-x="is initial
about:blank">initial <code>about:blank</code></span> <code>Document</code> in a
<code>frame</code> or <code>iframe</code>, since at the point of <span data-x="creating a new
browsing context">browsing context creation</span> which calls this algorithm, the container
relationship has not yet been established. (That happens in a subsequent step of <span>create
a new nested browsing context</span>.)</p>

<p>The consequence of this is that we the following steps do nothing, i.e., we do not fire an
asynchronous <code data-x="event-load">load</code> event on the container element for such
cases. Instead, a synchronous <code data-x="event-load">load</code> event is fired in a special
initial-insertion case in the <span>shared attribute processing steps for <code>iframe</code>
and <code>frame</code> elements</span>.</p>
</div>
</li>

<li><p>If <var>container</var> is an <code>iframe</code> element, then <span>queue an element
task</span> on the <span>DOM manipulation task source</span> given <var>container</var> to run
Expand Down Expand Up @@ -119020,7 +119082,7 @@ interface <dfn interface>HTMLFrameSetElement</dfn> : <span>HTMLElement</span> {
<li><p>If <var>element</var> has a <code undefined data-x="attr-frame-src">src</code> attribute
specified, or <var>initialInsertion</var> is false, then run the <span>shared attribute
processing steps for <code>iframe</code> and <code>frame</code> elements</span> given
<var>element</var>.</p></li>
<var>element</var> and <var>initialInsertion</var>.</p></li>
</ol>

<p>The <code>frame</code> element <span>potentially delays the load event</span>.</p>
Expand Down