Currently, we only preserve user gestures when the Promise returned by Document.requestStorageAccess is resolved. There are valid use cases for wanting to do so when the promise is rejected, such as allowing an embedded cross-origin frame to pop up a window that prompts the user to interact and login [0].

[0] https://github.com/whatwg/html/issues/3338#issuecomment-457373364

Tooru, do you have any guidance for how to do this? For reference, we did this for resolve handlers in bug 1491403. Thanks!

Is this specific to the API, or in general the user gesture should be preserved for rejection?
If latter, passing aPropagateUserInteraction in Promise::Reject [1] will solve.

afaik whether the promise is rejected or not doesn't matter in the JS engine side.

[1] https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/dom/promise/Promise.cpp#142

(In reply to Tooru Fujisawa [:arai] from comment #2)

Is this specific to the API, or in general the user gesture should be preserved for rejection?
If latter, passing aPropagateUserInteraction in Promise::Reject [1] will solve.

It is specific to this API. For this API, previously we ensured the resolve handlers are called with the user interaction flag set if it was set at the time the original Promise object was created. But we didn't do that for calling the reject handlers.

These promises are rejected in C++ using JS::RejectPromise(): https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/js/src/jsapi.cpp#4051.

afaik whether the promise is rejected or not doesn't matter in the JS engine side.

Do you mean the promises used in this API should already call their reject handlers with the user interaction flag set?

(In reply to :Ehsan Akhgari from comment #3)

(In reply to Tooru Fujisawa [:arai] from comment #2)

Is this specific to the API, or in general the user gesture should be preserved for rejection?
If latter, passing aPropagateUserInteraction in Promise::Reject [1] will solve.

It is specific to this API. For this API, previously we ensured the resolve handlers are called with the user interaction flag set if it was set at the time the original Promise object was created. But we didn't do that for calling the reject handlers.

Okay, let me look into it tonight.

These promises are rejected in C++ using JS::RejectPromise(): https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/js/src/jsapi.cpp#4051.

afaik whether the promise is rejected or not doesn't matter in the JS engine side.

Do you mean the promises used in this API should already call their reject handlers with the user interaction flag set?

in JS engine side, the "user input" flag is copied to the Promise created with then,
regardless of fulfillment or rejection:
https://searchfox.org/mozilla-central/search?q=copyUserInteractionFlagsFrom&path=

DOM promise side controls the propagation, by calling JS::SetPromiseUserInputEventHandlingState.
https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/dom/promise/Promise.cpp#103-111

and the different between fulfillment/rejection is only in dom::Promise::{Resolve,Reject}.
https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/dom/promise/Promise.cpp#113-143

If this specific case doesn't propagate the user input only for rejection case, I think that uses dom::Promise::Rejectsomewhere.
and somehow overriding the flag would work.

Actually you're right, I didn't know we handle resolve/reject exactly the same way. In this case we never call dom::Promise::Reject directly so we already do the right thing here. All that needs to be done is to just add a test case for this, I think.

Thanks for your help!

https://hg.mozilla.org/mozilla-central/rev/8abb91d7d1b7

Note to MDN writers:

I've added a note to the Fx67 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#DOM

I'm not sure if anny other docs additions are necessary, but let's check.

Thanks Chris. I think we'll also need to update the Storage Access API MDN docs. Specifically here: https://developer.mozilla.org/en-US/docs/Web/API/Document/requestStorageAccess#Syntax. Would you mind to do that?

(In reply to Steven Englehardt [:englehardt] from comment #10)

Thanks Chris. I think we'll also need to update the Storage Access API MDN docs. Specifically here: https://developer.mozilla.org/en-US/docs/Web/API/Document/requestStorageAccess#Syntax. Would you mind to do that?

Yup, we can do that when we finish off the docs task. No problem.

I have updated this section to acocunt for the change:

https://developer.mozilla.org/en-US/docs/Web/API/Document/requestStorageAccess#Return_value

Let me know if you are happy with the wording. Thanks!

LGTM, thanks Chris.