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.

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

Ignore a URL with an unsupported scheme #447

Merged
merged 2 commits into from Mar 26, 2018

Conversation

tomoyukilabs
Copy link
Contributor

@tomoyukilabs tomoyukilabs commented Mar 12, 2018

This PR addresses #446.

  • A URL with an unsupported scheme is ignored in constructing a PresentationRequest.

Preview | Diff

- Discard a URL with an unsupported scheme in constructing a PresentationRequest
tidoust pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 16, 2018
…ed (#9966)

Completes `PresentationRequest` constructor test with change [latest change made in Presentation API](w3c/presentation-api#447). The following tests are added:

- To check if an instance of `PresentationRequest` with URLs including one with an unsupported scheme is successfully constructed
- To check if `NotSupportedError` is thrown when only single or multiple URLs with unsupported schemes are specified
Copy link
Contributor

@mfoltzgoogle mfoltzgoogle left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. One editorial suggestion.

index.html Outdated
@@ -1182,13 +1187,19 @@ <h4>
<ol>
<li>Resolve <var>U</var> relative to the API base URL specified
by the <a>current settings object</a>, and add the resulting
absolute URL (if any) to <var>presentationUrls</var>.
absolute URL (if any, and if the resulting scheme is supported
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be easier to read if it were broken into three steps. Here is one way that could be done.

  1. Let A be absolute URL that is the result of resolving U relative to the API base URL specified by the current settings object.
  2. If the parse a URL algorithm failed, then throw a SyntaxError exception and abort all remaining steps.
  3. If A has a scheme supported by the controlling user agent, add A to presentationUrls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM. Thanks for your suggestion.

@tomoyukilabs
Copy link
Contributor Author

@mfoltzgoogle PTAL

Copy link
Contributor

@mfoltzgoogle mfoltzgoogle left a comment

Choose a reason for hiding this comment

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

LGTM with one minor typo

<li>Resolve <var>U</var> relative to the API base URL specified
by the <a>current settings object</a>, and add the resulting
absolute URL (if any) to <var>presentationUrls</var>.
<li>let <var>A</var> be an absolute URL that is the result of
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: s/let/Let/

@mfoltzgoogle mfoltzgoogle merged commit fcb49fd into w3c:gh-pages Mar 26, 2018
@mfoltzgoogle
Copy link
Contributor

Merging, can fix the typo in a followup commit.

mfoltzgoogle added a commit that referenced this pull request Mar 26, 2018
@tomoyukilabs
Copy link
Contributor Author

@mfoltzgoogle Many thanks for your review and fixing the typo.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 15, 2018
… unsupported scheme is ignored, a=testonly

Automatic update from web-platform-tests[presentation-api] check if a URL with an unsupported scheme is ignored (#9966)

Completes `PresentationRequest` constructor test with change [latest change made in Presentation API](w3c/presentation-api#447). The following tests are added:

- To check if an instance of `PresentationRequest` with URLs including one with an unsupported scheme is successfully constructed
- To check if `NotSupportedError` is thrown when only single or multiple URLs with unsupported schemes are specified

wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966
wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
… unsupported scheme is ignored, a=testonly

Automatic update from web-platform-tests[presentation-api] check if a URL with an unsupported scheme is ignored (#9966)

Completes `PresentationRequest` constructor test with change [latest change made in Presentation API](w3c/presentation-api#447). The following tests are added:

- To check if an instance of `PresentationRequest` with URLs including one with an unsupported scheme is successfully constructed
- To check if `NotSupportedError` is thrown when only single or multiple URLs with unsupported schemes are specified

wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966
wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966

UltraBlame original commit: 7b6feb16ae6fe5ee0889b819b649b2b41e003893
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
… unsupported scheme is ignored, a=testonly

Automatic update from web-platform-tests[presentation-api] check if a URL with an unsupported scheme is ignored (#9966)

Completes `PresentationRequest` constructor test with change [latest change made in Presentation API](w3c/presentation-api#447). The following tests are added:

- To check if an instance of `PresentationRequest` with URLs including one with an unsupported scheme is successfully constructed
- To check if `NotSupportedError` is thrown when only single or multiple URLs with unsupported schemes are specified

wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966
wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966

UltraBlame original commit: 7b6feb16ae6fe5ee0889b819b649b2b41e003893
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
… unsupported scheme is ignored, a=testonly

Automatic update from web-platform-tests[presentation-api] check if a URL with an unsupported scheme is ignored (#9966)

Completes `PresentationRequest` constructor test with change [latest change made in Presentation API](w3c/presentation-api#447). The following tests are added:

- To check if an instance of `PresentationRequest` with URLs including one with an unsupported scheme is successfully constructed
- To check if `NotSupportedError` is thrown when only single or multiple URLs with unsupported schemes are specified

wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966
wpt-commits: 1cbb9282ad842c48fff8a86e121ce7e33f6ca4a3
wpt-pr: 9966

UltraBlame original commit: 7b6feb16ae6fe5ee0889b819b649b2b41e003893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants