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

Should calling client.focus() imply preventDefault() #6

Open
fallaciousreasoning opened this issue Apr 1, 2019 · 2 comments
Open
Labels
future-api Issue discusses possible extensions to the current API.

Comments

@fallaciousreasoning
Copy link
Collaborator

My intuition here is that it should, as otherwise we open the path for strange behavior (I called client.focus() but the browser still opened a new window!).

This raises the question of whether preventDefault even needs to be part of the spec, as the event already MUST focus a client or show a notification in order to prevent default behavior. This means that a call to preventDefault is either:
a) Superfluous, we already know the app intends to preventDefault, they called client.focus() or displayed a notification or
b) A no-op, the app called preventDefault but they didn't focus a client/show a notification, so the browser does whatever it normally does.

For example, compare (explicit preventDefault):

self.addEventListener('launch', event => {
    event.waitUntil(async () => {
      // Call to prevent default. Be careful not to forget!
      event.preventDefault();

      const allClients = await clients.matchAll();
      // If there isn't one available, open a window.
      if (allClients.length === 0) {
        const client = clients.openWindow('/');
        client.postMessage(event.files);
        client.focus();
        return;
      }

      const client = allClients[0];
      client.postMessage(event.files);
      client.focus();
    }());
  });

and (with preventDefault implicit)

self.addEventListener('launch', event => {
    event.waitUntil(async () => {
      const allClients = await clients.matchAll();
      // If there isn't one available, open a window.
      if (allClients.length === 0) {
        const client = clients.openWindow('/');
        client.postMessage(event.files);
        client.focus();
        return;
      }

      const client = allClients[0];
      client.postMessage(event.files);
      client.focus();
    }());
  });

@raymeskhoury

@raymeskhoury
Copy link
Collaborator

I think you're probably right. Maybe we don't need preventDefault.

raymeskhoury added a commit that referenced this issue Jun 3, 2019
@fallaciousreasoning
Copy link
Collaborator Author

Recently, it was raised that it may be difficult to determine which handler has called focus/open/showNotification, so if we continue down this avenue we may need the call to preventDefault after all.

@alancutter alancutter added the future-api Issue discusses possible extensions to the current API. label Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future-api Issue discusses possible extensions to the current API.
Projects
None yet
Development

No branches or pull requests

3 participants