w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
578 stars 50 forks source link

`sidePanel` API: support "one instance per tab" in `openPanelOnActionClick` #515

Open fregante opened 6 months ago

fregante commented 6 months ago

The default behavior of the sidePanel API seems to be to share a single document per window. You open the sidebar and it persists while switching tabs. ⚠️

This is great for "global" extensions or those that have a UI with limited complexity (i.e. they don't require fetching and displaying data based on the current page)

However in our extension the sidebar shows content related to the current page/contentScript, so we'd rather have a dedicated sidebar per tab. This means we prefer the sidebar to disappear when switching to tabs that don't need it.

The good news

Chrome actually already allows "one sidebar instance per tab", but it requires calling setOptions on every tab, with some complexity and drawbacks.

🖼️ Demo gif

![all](https://github.com/w3c/webextensions/assets/1402241/eacd5913-1229-48dd-9c74-db52d638ac48)

Proposal

chrome.sidePanel.setPanelBehavior({
    openPanelOnActionClick: true,
+   linkToTab: true,
});

Current solution

void chrome.sidePanel.setPanelBehavior({ openPanelOnActionClick: false });

// Disable by default, so that it can be enabled on a per-tab basis
void chrome.sidePanel.setOptions({
  enabled: false,
});

chrome.action.onClicked.addListener(async (tab) => {
  // Simultaneously define, enable, and open the side panel
  void chrome.sidePanel.setOptions({
    enabled: true,

    // Link to tab
    tabId,

    // Help sidebar.html know which tab it's connected to
    path: "sidebar.html?tabId=" + tab.id,
  });
  await chrome.sidePanel.open({ tabId });
});

Drawbacks for the current solution

This is somewhat feasible but it comes with some drawbacks and complexity:

tophf commented 5 months ago

Did you try chrome.tabs.onActivated inside the side panel and add/remove DOM elements or toggle visibility accordingly? If it doesn't flicker, the overall memory consumption would be less by ~20MB per tab probably. As for the other issues you mentioned, they should be fixed anyway regardless of this proposal.

fregante commented 5 months ago

I think you're referring to having one sidebar instance and then changing its content depending on the tab.

It can work in some cases but here preserving the state between tab changes can be difficult; the side panel here contains a complex app (think multi-step forms, iframes, etc) that can't be easily unloaded and restored without side effects.

tophf commented 5 months ago

AFAIK it should be possible. Which side effects specifically?

fregante commented 5 months ago

Which side effects specifically?

What happens when you remove these elements from the DOM:

Removing/reattaching them on tab change will trigger HTTP requests and it will lose state, e.g. navigation in the iframe. These are just the most prominent examples and not an exhaustive list.

AFAIK it should be possible

It is possible, but it moves the complexity to the app itself, for example in a React app you'd have to specifically try to preserve the entire DOM for every relevant tab.

Even for simple sidebars, having a dedicated document/instance per tab greatly simplifies management.

Since having multiple sidebars is already possible, I'd always prefer that over having to deal with tab/content changes.

tophf commented 5 months ago

Well, don't remove them, simply toggle their visibility. I don't see anything complex in having multiple apps in one document.

tophf commented 5 months ago

You can also put the entire app inside an iframe to isolate the globals/whatever and have an iframe per tab, then toggle the iframe visibility. You'll be able to reuse resources and data between all of them through parent, so it'll consume less resources than separate pages.

tophf commented 5 months ago

But if your proposal is easy to implement for browser makers I agree it makes sense for the sake of consistency of the API.

fregante commented 5 months ago

Let's stick to the proposal, because that solution is in no way better than just loading dedicated sidebars (which already works, without altering the app)

Also what you're suggesting just doesn't match the requirement: the sidebar must close when switching to tab that didn't open it.

dotproto commented 5 months ago
  • there's no sidePanel.close() API, so to close the sidebar in a tab, you'd have to set enabled: false, which removes it from the dropdown

Have you tried calling window.close() inside the side panel window?

fregante commented 5 months ago

@dotproto That's really useful, thanks! I opened a dedicated issue for that: