Change a variety of entry settings object uses#1541
Merged
Conversation
This was referenced Jul 12, 2016
| </li> | ||
|
|
||
| <li><p>Let <var>targetRealm</var> be this <code>History</code> object's <span>relevant settings | ||
| object</span>'s <span data-x="environment settings object's realm">Realm</span>.</p></li> |
Member
There was a problem hiding this comment.
Removing this seems wrong. We could change it to use "relevant Realm" though as we've done for another instance.
Member
Author
There was a problem hiding this comment.
This is actually just a redundant step. Earlier in the algorithm a very similar step already appears.
Member
|
LGTM, assuming your testing is correct. |
Since this is a same origin-domain check, we can use any settings object, as they are all same origin-domain. Part of #1431.
This updates the origin check in pushState/replaceState to use the origin of the document of the relevant History object, instead of that of the entry settings object. This more correctly matches 2/3 open source browsers: - https://chromium.googlesource.com/chromium/src/+/c21f0b11ac83ea970d0eaf6a0b223d48a32a4b32/third_party/WebKit/Source/core/frame/History.cpp#234 - https://github.com/WebKit/webkit/blob/0ee7b606dbf35d9688c15b19b1a83ec1ff242cd7/Source/WebCore/page/History.cpp#L150 (Gecko does no such security check). It also helps with #1431. While there, cleaned up some redundant steps and tightened wording.
The test at https://url-parsing-entry-gjqursujea.now.sh/entry/entry.html reveals that Firefox at least uses relevant for URL parsing. Boris says that Firefox does something nonsensical for the origin check and that he would prefer relevant or current. Code inspection reveals that Blink and WebKit use relevant for both URL parsing and origin checking: - https://chromium.googlesource.com/chromium/src/+/ee94bde91c72a046bec15436d56cfe32bb0e524c/third_party/WebKit/Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp#71 - https://github.com/WebKit/webkit/blob/83624b838d4fa81df77060c51b09587169f8ff19/Source/WebCore/Modules/navigatorcontentutils/NavigatorContentUtils.cpp#L109 Edge does not support these methods. Helps with #1431.
Since this is a same origin-domain check, we can use any settings object, as they are all same origin-domain. Part of #1430.
7699b11 to
a6a1b71
Compare
domenic
added a commit
that referenced
this pull request
Jul 21, 2016
In #1541, a few checks on the "currently-executing constructor" were added. The pull request review revealed that this phrase was not sufficiently precise, so it was replaced with "active function object", a term defined in the JavaScript specification. However, this was only done in one of the three places the imprecise term appears. This tidies up the other two.
annevk
pushed a commit
that referenced
this pull request
Jul 21, 2016
In #1541, a few checks on the "currently-executing constructor" were added. The pull request review revealed that this phrase was not sufficiently precise, so it was replaced with "active function object", a term defined in the JavaScript specification. However, this was only done in one of the three places the imprecise term appears. This tidies up the other two.
annevk
pushed a commit
that referenced
this pull request
Jul 25, 2016
In #1541, a few checks on the "currently-executing constructor" were added. The pull request review revealed that this phrase was not sufficiently precise, so it was replaced with "active function object", a term defined in the JavaScript specification. However, this was only done in one of the three places the imprecise term appears. This tidies up the other two.
alice
pushed a commit
to alice/html
that referenced
this pull request
Jan 8, 2019
In whatwg#1541, a few checks on the "currently-executing constructor" were added. The pull request review revealed that this phrase was not sufficiently precise, so it was replaced with "active function object", a term defined in the JavaScript specification. However, this was only done in one of the three places the imprecise term appears. This tidies up the other two.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is all progress on #1431, within the bounds of what is web compatible and which I've been able to somewhat easily test.