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

Set request's reload-navigation flag when the navigation is reload-trigerred #3592

Merged
merged 5 commits into from
Apr 27, 2018
Merged

Set request's reload-navigation flag when the navigation is reload-trigerred #3592

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
247a69b
fix
yutakahirano Mar 27, 2018
3b6a361
fix
yutakahirano Mar 27, 2018
afd7729
fix
yutakahirano Mar 29, 2018
6f38775
fix
yutakahirano Mar 29, 2018
1d56071
fix
yutakahirano Apr 26, 2018
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
8 changes: 7 additions & 1 deletion source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2819,6 +2819,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li><dfn data-x="concept-request-nonce-metadata" data-x-href="https://fetch.spec.whatwg.org/#concept-request-nonce-metadata">cryptographic nonce metadata</dfn></li>
<li><dfn data-x="concept-request-integrity-metadata" data-x-href="https://fetch.spec.whatwg.org/#concept-request-integrity-metadata">integrity metadata</dfn></li>
<li><dfn data-x="concept-request-parser-metadata" data-x-href="https://fetch.spec.whatwg.org/#concept-request-parser-metadata">parser metadata</dfn></li>
<li><dfn data-x="concept-request-reload-navigation-flag" data-x-href="https://fetch.spec.whatwg.org/#concept-request-reload-navigation-flag">reload-navigation flag</dfn></li>
</ul>
</li>
</ul>
Expand Down Expand Up @@ -81813,6 +81814,10 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
data-x="concept-request">request</span> whose <span data-x="concept-request-url">url</span> is
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

With this change the algorithm may set a flag on the request. I thought that call-sites might be surprised by that behavior, and made a copy in case. If that's not the case I can remove this statement.

Copy link
Member

Choose a reason for hiding this comment

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

I think all the callers of navigate don't hold onto the objects they pass. And I think only <form> passes request objects, so that should be easy to verify.

Copy link
Member

Choose a reason for hiding this comment

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

(Which also means that location.reload() doesn't, which means this patch doesn't do what it wants I think.)

Copy link
Member Author

@yutakahirano yutakahirano Mar 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, what kind of object does location.reload() pass? If it's neither URL nor request, is it handled by "If resource is a request whose url's scheme is a fetch scheme..." statement?

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry, I think location.reload() does pass a URL at the moment, but I think it should pass a "session history entry" since it's supposed to take into account whether the document was POSTed and such.

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 deleted this statement.

<var>resource</var>.</p></li>

<li><p>If <var>resource</var> is a <span data-x="concept-request">request</span> and this is
a <dfn>reload-triggered navigation</dfn>, then set <var>resource</var>'s <span
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean that a manual refresh wouldn't set the flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, what do you mean by "manual refresh"?

Copy link
Member

Choose a reason for hiding this comment

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

A user-initiated refresh.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I put the statement in a wrong place. Fixed.

data-x="concept-request-reload-navigation-flag">reload-navigation flag</span>.

<li id="sandboxLinks">
<p>If the <span>source browsing context</span> is not <span>allowed to navigate</span>
<var>browsingContext</var>, then:</p>
Expand Down Expand Up @@ -81850,7 +81855,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<span>prompt to unload</span> algorithm.</p></li>
<!-- https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=1946 to 1955 -->

<li id="navigate-fragid-step"><p>If this is not a <dfn>reload-triggered navigation</dfn>,
<li id="navigate-fragid-step"><p>If this is not a <span>reload-triggered navigation</span>
Copy link
Member

Choose a reason for hiding this comment

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

Missing comma here

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

<var>resource</var> is a <span data-x="concept-request">request</span>, <var>resource</var>'s
<span data-x="concept-request-url">url</span> <span data-x="concept-url-equals">equals</span>
<var>browsingContext</var>'s <span>active document</span>'s <span
Expand Down Expand Up @@ -121493,6 +121498,7 @@ INSERT INTERFACES HERE
Yoav Weiss,
Yonathan Randolph,
Yury Delendik,
平野裕 (Yutaka Hirano)
Yuzo Fujishima,
Zhenbin Xu,
&#24352;&#26234;&#24378; (Zhiqiang Zhang),
Expand Down