fix storeTabData caching wrong frame target values during prerendering

[fungairino]

Fixes pixiebrix/pixiebrix-extension#8283

The storeTabData was caching the wrong frame value with its once usage when the browser prerenders the page (ex. the user starts typing the url in the search bar and the page is loaded (i.e. prerendered) in the background.

This change fixes the issue by waiting for the prerenderingchange event if the document is currently in prerendering state.

I tested these changes by running npm link and then in pixiebrix-extension ran npm link webext-messenger. I built the extension and tested the above-linked bug with a mod that reproduces the error. With this new change, the issue no longer is present.

Reviewer: @twschiller

Tag: @fregante

New dependency, p-event. MIT License
https://www.npmjs.com/package/p-event

[fregante]

What happens if both calls to storeTabData are made while the tab is prerendering? The content script will be in a broken state: no tab data will ever be requested again, but this data is a hard requirement in the content script.

The only advised change here would look like this instead:

async function storeTabData() {
  if (document.prerendering) {
    await pEvent(document, 'prerenderingchange')
  }
  //continue

• https://developer.mozilla.org/en-US/docs/Web/API/Document/prerenderingchange_event
• https://github.com/sindresorhus/p-event

[fungairino]

@fregante

What happens if both calls to storeTabData are made while the tab is prerendering? The content script will be in a broken state: no tab data will ever be requested again, but this data is a hard requirement in the content script.

I'm not sure I understand. What two calls are you referring to?

It seems to me that with this change we will make sure to refetch the tab data if/when it's needed again after the prerendered tab is activated.

[fregante]

storeTabData is only ever called twice in this package, you can search the two calls. The first one immediately on the first run, and it's blocked by this new prerendering check, the second one is called in the ping response handler. I reckon they could both happen while the document is prerendering, and therefore never save the tab data.

It should just wait until the document explicitly exits the prerendering stage instead.

[fungairino]

@fregante's suggested solution is simpler and also works, so I'll go with this approach!

async function storeTabData() {
  if (document.prerendering) {
    await pEvent(document, 'prerenderingchange')
  }

[twschiller]

@fungairino you might not need the if statement. The docs say that it might fire immediately if the document is already rendered: https://developer.mozilla.org/en-US/docs/Web/API/Document/prerenderingchange_event#examples

[fungairino]

prerendering isnt a widely supported browser feature yet, so I'm leaning towards keeping the check to be more explicit. Especially since the linked example also still checks for the prerendering attribute.