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

Web: cannot go fullscreen or copy via Quick Open from Firefox #83288

Closed
bpasero opened this issue Oct 25, 2019 · 3 comments
Assignees
Milestone

Comments

@bpasero
Copy link
Member

@bpasero bpasero commented Oct 25, 2019

Looks like:

  • we need to support the moz-prefix
  • it still fails when doing that for me:

Request for fullscreen was denied because Element.requestFullscreen() was not called from inside a short running user-generated event handler.

@bpasero bpasero changed the title Web: cannot go fullscreen from Firefox Web: cannot go fullscreen via command from Firefox Oct 25, 2019
@rebornix

This comment has been minimized.

Copy link
Member

@rebornix rebornix commented Oct 25, 2019

... was not called from inside a short running user-generated event handler

This is related to how a command is being executed. For example, Toggle Full Screen from Menu or keyboard shortcuts works, run the same command from Quick Open doesn't and the reason for that is we do setTimeout to let Quick Open close first. The setTimeout tricks Firefox and it longer thinks it's from a user-generate event ;(

// Use a timeout to give the quick open widget a chance to close itself first
setTimeout(async () => {
if (action && (!(action instanceof Action) || action.enabled)) {
try {
this.telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: action.id, from: 'quick open' });
const promise = action.run();
if (promise) {
try {
await promise;
} finally {
if (action instanceof Action) {
action.dispose();
}
}
}
} catch (error) {
this.onError(error);
}
} else {
this.notificationService.info(localize('actionNotEnabled', "Command '{0}' is not enabled in the current context.", this.getLabel()));
}
}, 50);

So this affects everything that requires a user generated event, for example Copy and Paste. If we run Copy from the Command Palette, it fails for the same reason. I like how Firefox tries its best to be safe but then we need to look into how many actions/commands are affected (IMO no a lot).

@rebornix rebornix changed the title Web: cannot go fullscreen via command from Firefox Web: cannot go fullscreen or copy via Quick Open from Firefox Oct 25, 2019
@rebornix rebornix added the quick-pick label Oct 25, 2019
@bpasero

This comment has been minimized.

Copy link
Member Author

@bpasero bpasero commented Oct 26, 2019

@rebornix good finding. If you add the code for supporting fullscreen for Firefox, I can look into fixing quick open. We may have to add a firefox check there...

@bpasero

This comment has been minimized.

Copy link
Member Author

@bpasero bpasero commented Oct 26, 2019

Ah I get it, fullscreen works already, just not from quick pick 👍

@bpasero bpasero assigned bpasero and unassigned rebornix Oct 26, 2019
@bpasero bpasero added this to the October 2019 milestone Oct 26, 2019
@bpasero bpasero closed this in a97d52c Oct 28, 2019
@joaomoreno joaomoreno added the verified label Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.