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

Proposal: Complete privileged page process even after closing #315

Open
erosman opened this issue Nov 1, 2022 · 7 comments
Open

Proposal: Complete privileged page process even after closing #315

erosman opened this issue Nov 1, 2022 · 7 comments
Labels
neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox supportive: safari Supportive from Safari

Comments

@erosman
Copy link

erosman commented Nov 1, 2022

Purpose

Reduce dependency on background scripts in extensions

Preamble

Privileged pages (i.e. action (browserAction) or options) usually need to run multiple async operations based on user actions.

Since the operation may be interrupted if the page is closed, developers often pass the process to a persistent background page to complete, which involves adding runtime.sendMessage & runtime.onMessage.

A storage.onChanged listeners is often used in background pages to listen to changes from the aforementioned privileged pages in order to initiate subsequent processes.

In non-persistent manifest v3 background service worker or event page, in addition to restarting the page, often other supplementary operations are needed (e.g. StorageArea.get).

Proposal

IF it would have been possible to complete the process that was started on a privileged page (i.e. action or options) even after the page is closed, then there would not be a need to pass the operation to a background page and add extra listeners.

I can imagine that would remove considerable number of background page/SW operations. I can see many extensions not needing a background page as a result, or needing a background page only to run once on start-up, or have a background page with fewer listeners.

@tophf
Copy link

tophf commented Nov 1, 2022

Interesting idea.

This would probably require a hard-coded timeout e.g. 30 seconds, same as service worker. Closing would just hide the window without changing anything else, so that if the user re-opens the page while it's still alive it would be shown instantly in its current state. Some DOM event should be dispatched in this case, I don't think the existing Lifecycle events would suit.

Another solution for this problem may be to respect onbeforeunload listener in these pages so that the browser wouldn't silently close the page but instead will show a confirmation, just like it does for normal pages.

@carlosjeurissen
Copy link
Contributor

Would this also allow users to open the popup again in its current state? This would help users to easily pick-up where they left off in cases of the popup being closed accidentality or due to a focusout reason.

@erosman
Copy link
Author

erosman commented Nov 9, 2022

Would this also allow users to open the popup again in its current state?

The proposal is to complete the process and then discard the (hidden) page normally.

📌 N.B. We are only concerned with completing the process that resulted from the user-action while the privileged page was open.

Let's consider some basic examples.

storage

A browserAction popup (or options page) with a button that if clicked gets some data from tabs.query() and saves to storage.

document.querySelector('button').addEventListener('click', onClick);

function onClick() {
  browser.tabs.query({currentWindow: true, active: true})
  .then(tabs => browser.storage.local.set({url: tabs[0].url)})
  .catch(console.error);
}

fetch

A browserAction popup with a button that if clicked would get user IP and display via notifications API.

document.querySelector('button').addEventListener('click', onClick);

function onClick() {
  fetch('https://api.ipify.org?format=json')
  .then(response => response.json())
  .then(data => notify(data.ip))
  .catch(error => notify(error.message));
}

function notify(message, id = '') {
  browser.notifications.create(id, {
    type: 'basic',
    iconUrl: '/image/icon.svg',
    title: 'Your IP',
    message
  });
}

If user closes the popup after clicking the button, the execution is terminated. Therefore, developers are forced to pass the process to the background script.

@Sxderp
Copy link

Sxderp commented Nov 16, 2022

Would this also allow users to open the popup again in its current state? This would help users to easily pick-up where they left off in cases of the popup being closed accidentality or due to a focusout reason.

I think the initial task completion issue is more important. Being able to reopen with existing state might be a future goal.

@oliverdunk
Copy link
Member

We hit this at 1Password where copying data to the clipboard was failing because we forgot to await navigator.clipboard.writeText before closing the popup. While arguably this was a bug on our side (or even a Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1073225) I think it shows an example of how keeping the popup alive in the background for a few seconds may help developers.

@erosman
Copy link
Author

erosman commented Nov 25, 2022

On a side note ...

There are Chrome bugs which further complicate the issue of messaging between privileged page and background service worker:

@xeenon xeenon added the supportive: safari Supportive from Safari label Dec 8, 2022
@Rob--W Rob--W added neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox labels Dec 8, 2022
@erosman
Copy link
Author

erosman commented Dec 10, 2022

Subsequent to 2022-12-08 meeting notes, an API to temporarily prevent the auto-closure of browserAction popup might be the simplest method to implement.

An API would eliminate the complications of:

  • Keeping the DOM alive after closure
  • New vs old popup (commented by tomislav)
  • Implementing a time-based delay (commented by timothy)

📌 In case of privileged tabbed pages (e.g. options, sidebar) Window: beforeunload event can be used.

API Implementation for popup

Here are some implementation ideas for the API. I think the 2nd option, the async delayAutoHide, might be a better option.

1. Disable/Enable Auto-Hide for this popup

  • It might need to be synchronous
  • Additional 2 methods are needed

Example:

browser.action.disableAutoHide();

await browser.storage.local.set({url: '....'});

browser.action.enableAutoHide();

2. Delay until return for this popup

  • It is asynchronous
  • Only 1 method is needed

Example:

browser.action.delayAutoHide(someFunction);

async function someFunction() {
  await browser.storage.local.set({url: '....'});
};

@dotproto dotproto removed the agenda Discuss in future meetings label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neutral: chrome Not opposed or supportive from Chrome neutral: firefox Not opposed or supportive from Firefox supportive: safari Supportive from Safari
Projects
None yet
Development

No branches or pull requests

8 participants