Reject with DomException or never reject #118
Conversation
bvandersloot-mozilla
changed the title
There was a problem hiding this comment.
I don't think you can just leave the promise dangling like that, that will generate both memory leaks as well as developer experience problems for legitimate implementations.
I could imagine resolving #60 either through a fixed 2 min delay as in FedCM or simply an advisory in Privacy considerations that the UA should add jitter to its response that makes it harder to distinguish human users.
Let's do it in a separate PR, though.
storage-access.bs
Outdated
| @@ -202,19 +202,19 @@ When invoked on {{Document}} |doc|, the <dfn export method for=Document><code>re | |||
|
|
|||
| 1. Let |p| be [=a new promise=]. | |||
| 1. If |doc| is not [=Document/fully active=], then [=reject=] |p| with an "{{InvalidStateError}}" {{DOMException}} and return |p|. | |||
| 1. If this algorithm was invoked when |doc|'s {{Window}} object did not have [=transient activation=], [=reject=] and return |p|. | |||
| 1. If this algorithm was invoked when |doc|'s {{Window}} object did not have [=transient activation=], [=reject=] |p| with a "{{NotAllowedError}}" {{DomException}} and return |p|. | |||
There was a problem hiding this comment.
Casing is DOMException.
Any reason not use a generic TypeError? (That's JavaScript's catch-all error, despite "type" in the name.)
There was a problem hiding this comment.
I will fix the casing if we settle on DOMExceptions.
I prefer it because it matches the type from line 204, so the promise will always reject with the same type. I also prefer more specific error types where possible and "the request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission" sounds like a good fit over the generic error.
There was a problem hiding this comment.
Not using TypeError seems fine to me, FWIW. I'll add that I'm hoping to revisit error types for common failures across specs and may come to a different recommendation later. I agree that intuitively NotAllowedError seems best for activation so I'm fine with merging this.
|
@johannhof |
Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
This is reflected in the latest version of the spec, merged by privacycg/storage-access#118 . Differential Revision: https://phabricator.services.mozilla.com/D158692 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1779575 gecko-commit: 4a2fa09ac7c8c56b8430418e56001b777be5b454 gecko-reviewers: anti-tracking-reviewers, smaug, timhuang
… r=anti-tracking-reviewers,smaug,timhuang This is reflected in the latest version of the spec, merged by privacycg/storage-access#118 . Differential Revision: https://phabricator.services.mozilla.com/D158692
This is reflected in the latest version of the spec, merged by privacycg/storage-access#118 . Differential Revision: https://phabricator.services.mozilla.com/D158692 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1779575 gecko-commit: 4a2fa09ac7c8c56b8430418e56001b777be5b454 gecko-reviewers: anti-tracking-reviewers, smaug, timhuang
… r=anti-tracking-reviewers,smaug,timhuang This is reflected in the latest version of the spec, merged by privacycg/storage-access#118 . Differential Revision: https://phabricator.services.mozilla.com/D158692
I am both rejecting with a "{{NotAllowedError}}" {{DomException}} in all contexts where we reject undefined and no longer rejecting where we don't want to leak user choice timing. (#60 and #37)
Preview | Diff