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

Use [SecureContext] instead of manually throwing an exception #143

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

romandev
Copy link
Member

@romandev romandev commented Apr 2, 2018

No description provided.

@kenchris
Copy link
Contributor

kenchris commented Apr 3, 2018

@alexshalamov are you fine with this? I assume this is the same you do in generic sensors?

index.html Outdated
@@ -1462,6 +1463,9 @@ <h2>The <dfn>NFCRecordType</dfn> enum</h2>
cancelable Promises</a> used with <code>push()</code> and
<code>watch()</code>, respectively.
</p>
<p class="note">
Copy link

@alexshalamov alexshalamov Apr 3, 2018

Choose a reason for hiding this comment

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

I'm not sure whether this extra information adds something, localhost considered to be secure, that is sufficient for development needs, thus UA don't need to handle special case.

index.html Outdated
@@ -1765,18 +1769,6 @@ <h2>The <dfn>NFCWatchMode</dfn> enum</h2>
Return a new <a><code>Promise</code></a> <var>promise</var>, and
then continue running this algorithm <a>in parallel</a>.
</li>
<li>
If any exception occurs while running these steps, reject

Choose a reason for hiding this comment

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

This block is not related to secure context, we can keep it as it is.

index.html Outdated
@@ -2656,18 +2640,6 @@ <h2>The <dfn>NFCWatchMode</dfn> enum</h2>
Return <var>promise</var> and continue the following steps
<a>in parallel</a>.
</li>
<li>
If any exception occurs while running these steps, reject

Choose a reason for hiding this comment

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

Same as above, let's remove custom handling of secure context and remove only following blocks:

-          <li>
-            If the <a>incumbent settings object</a> is not a
-            <a>secure context</a>, reject <var>promise</var> with
-            <code>"<a>SecurityError</a>"</code> and abort these steps.
-            <div class="note">
-              Browsers may ignore this rule for development purposes only.
-            </div>
-          </li>

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 3, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
@romandev
Copy link
Member Author

romandev commented Apr 3, 2018

@alexshalamov Thank you for review. I addressed your comments.

@alexshalamov
Copy link

@romandev Thanks!

@alexshalamov alexshalamov merged commit e5bccd7 into w3c:gh-pages Apr 4, 2018
romandev added a commit to romandev/web-nfc that referenced this pull request Apr 5, 2018
This change adds two extended attributes to navigator.nfc for the
following reasons.
  - [SecureContext]
    - We already added this to `NFC` interface[1] but it was not enough.
      Should provide feature-detection[2].
  - [SameObject]
    - There is no reason to create a new object whenever accessing
      `navigator.nfc`.

[1] w3c#143
[2] https://chromium-review.googlesource.com/c/chromium/src/+/989537#message-eab848bd9bc7fba8b163630d3731c9d851a87ac3
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 5, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 5, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 6, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamov@intel.com>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#548744}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 6, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamov@intel.com>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#548744}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 6, 2018
Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamov@intel.com>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#548744}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 15, 2018
…anual check, a=testonly

Automatic update from web-platform-testsWebNFC: Use [SecureContext] instead of manual check

Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamov@intel.com>
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Cr-Commit-Position: refs/heads/master@{#548744}

wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284
wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…anual check, a=testonly

Automatic update from web-platform-testsWebNFC: Use [SecureContext] instead of manual check

Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamovintel.com>
Commit-Queue: Jinho Bang <jinho.bangsamsung.com>
Cr-Commit-Position: refs/heads/master{#548744}

wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284
wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284

UltraBlame original commit: 647daf60d4f78cbb3e8c004b825da99f74abcb84
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…anual check, a=testonly

Automatic update from web-platform-testsWebNFC: Use [SecureContext] instead of manual check

Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamovintel.com>
Commit-Queue: Jinho Bang <jinho.bangsamsung.com>
Cr-Commit-Position: refs/heads/master{#548744}

wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284
wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284

UltraBlame original commit: 647daf60d4f78cbb3e8c004b825da99f74abcb84
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…anual check, a=testonly

Automatic update from web-platform-testsWebNFC: Use [SecureContext] instead of manual check

Until now, we had to use IsSecureContext() to check whether an execution
context is a secure context manually. But we can use [SecureContext]
instead of the manual check now.

Related spec change:
  w3c/web-nfc#143
  w3c/web-nfc#144

Bug: none
Change-Id: I1396470cc57aeba137ddba65d1f29eb58cf9cf9b
Reviewed-on: https://chromium-review.googlesource.com/989537
Reviewed-by: Alexander Shalamov <alexander.shalamovintel.com>
Commit-Queue: Jinho Bang <jinho.bangsamsung.com>
Cr-Commit-Position: refs/heads/master{#548744}

wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284
wpt-commits: 11740d8ae8381b28655ef3811a3fc8afd3a9ded1
wpt-pr: 10284

UltraBlame original commit: 647daf60d4f78cbb3e8c004b825da99f74abcb84
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.

4 participants