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 InvalidStateError instead of SecurityError for fully active checks #112

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

johannhof
Copy link
Member

@johannhof johannhof commented Sep 2, 2022

@mreichhoff I noticed that InvalidStateError seems to be the most common error state for non-fully active checks, is there any particular reason we picked a SecurityError in Chromium for this?


Preview | Diff

@mreichhoff
Copy link
Contributor

I used SecurityError because I happened to be linked to the history go API, which also uses SecurityError, and I viewed both APIs as sensitive.
https://html.spec.whatwg.org/#dom-history-go

the relevant line:

If document is not fully active, then throw a "SecurityError" DOMException.

I certainly don't have a strong opinion, however. If InvalidStateError is more common, that seems like a better approach.

@johannhof
Copy link
Member Author

Yeah, I filed w3ctag/design-principles#395 for having some guidance on which errors are to be used in this case. I wasn't aware of the usage in the History API, so now I understand why we originally chose SecurityError. However, it looks like the majority of APIs (including the proposed history API successor) use InvalidStateError now, so I'll go ahead and merge this.

Thanks!

@johannhof johannhof merged commit 8c7989f into privacycg:main Sep 8, 2022
@johannhof johannhof deleted the invalid-state branch September 8, 2022 07:03
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 28, 2022
See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
aarongable pushed a commit to chromium/chromium that referenced this pull request Sep 29, 2022
See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 29, 2022
See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Sep 29, 2022
See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 13, 2022
…or with InvalidStateError, a=testonly

Automatic update from web-platform-tests
Replace SAA non-fully active SecurityError with InvalidStateError

See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}

--

wpt-commits: a1de4a7cd450ae83127f17c4dcb5aff1f0e1d83d
wpt-pr: 36144
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}
NOKEYCHECK=True
GitOrigin-RevId: 803dae7978e9a0fb88489e41f71e7a84f1b2bd78
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Oct 19, 2022
…or with InvalidStateError, a=testonly

Automatic update from web-platform-tests
Replace SAA non-fully active SecurityError with InvalidStateError

See discussion on the related pull request:
privacycg/storage-access#112

Bug: 1354256
Change-Id: Ie2ac7890ae1039feca28b63239f02e476070e813
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3926236
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Matt Reichhoff <mreichhoff@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1053043}

--

wpt-commits: a1de4a7cd450ae83127f17c4dcb5aff1f0e1d83d
wpt-pr: 36144
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.

2 participants