1817872 - browser.tabs.onCreated should wait for tab to fully load before notifying extensions (supernova)

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[Mark Banner (:standard8)]

In a WebExtension, try and doing something like:

browser.tabs.onCreated.addListener(async (tab) => {
 await browser.messageDisplay.getDisplayedMessage(tab.id)
});

This fails with:

can't access property "getSelectedMsgHdrs", nativeTab.chromeBrowser.contentWindow.gDBView is undefined in ext-messageDisplay.js

This is because the new code for tabs is being loaded in a browser that may not have loaded yet. The onCreated listener is getting fired too early. Something like this seems to be enough when I tried in a WebExtension experiment API:

  return new Promise((resolve) => {
    if (win.document.readyState == "complete") {
      resolve();
    } else {
      win.addEventListener(
        "load",
        () => {
          resolve();
        },
        { once: true }
      );
    }
  });

Comment 1

[John Bieling (:TbSync)]

I personally have never understood, why the specs for onCreated did not wait for load. Nobody wants to do something with a tab before it has fully loaded. If this worked for you for message tabs prior to Supernova, I assume this was luck :-). Also content tabs are returned before the content has loaded.

I think I should not change the default behaviour, but I can add a parameter to tabs.onCreated.addListerner() which will ensure you get the tab only after it has fully loaded.

Edit: No, I will guard the messageDisplay API to wait until the tab is fully loaded, like we do it for the compose API already (partially)

Comment 2

[John Bieling (:TbSync)]

I removed the regression keyword and marked this as an enhancement.

Comment 3

[John Bieling (:TbSync)]

I tested this code:

browser.tabs.onCreated.addListener(async (tab) => {
  console.log(tab.status, tab.type);
  if (tab.type == "messageDisplay") {
    console.log("MSG");
    let msg = await browser.messageDisplay.getDisplayedMessage(tab.id)
    console.log(msg);
  }
});

I get a "completed" status, but no msg. So that is inconsistent and wrong.

Comment 4

[Thomas D. (:thomas8)]

Adding [Supernova] to Whiteboard. 20230406_1102

Comment 5

[John Bieling (:TbSync)]

Attachment: Bug 1817872 - Return the correct load status for message tabs. r=standard8,darktrojan

Comment 6

[John Bieling (:TbSync)]

What I did in the patch is fixing the wrong reporting of the tab being completely loaded.

I did not change the behaviour of tabs.onCreated to report tabs before they are loaded. I added a test to verify the fixed behaviour, which one could use in add-ons as well (still have to wait for the try-run):

      let messageTab = await new Promise(resolve => {
        let createListener = tab => {
          browser.tabs.onCreated.removeListener(createListener);
          if (tab.status == "complete") {
            resolve(tab);
          } else {
            browser.tabs.onUpdated.addListener(updateListener, {
              tabId: tab.id,
            });
          }
        };
        let updateListener = (tabId, changeInfo, tab) => {
          if (changeInfo?.status == "complete") {
            browser.tabs.onUpdated.removeListener(updateListener);
            resolve(tab);
          }
        };
        browser.tabs.onCreated.addListener(createListener);
        browser.messageDisplay.open({
          location: "tab",
          messageId: <some message id>,
        });
      });
      // We should now be able to get the message.
      let message = await browser.messageDisplay.getDisplayedMessage(
        messageTab.id
      );

Note: If you use the messageDisplay.onMessageDisplayed event, you get the tab and message directly.
https://webextension-api.thunderbird.net/en/stable/messageDisplay.html#onmessagedisplayed

Comment 7

[John Bieling (:TbSync)]

In a second patch, I will add a guard for messageDisplay.getDisplayedMessage() to wait until the tab is fully loaded, before returning the message. We do something similar for compose.getComposeDetails().

Comment 8

[John Bieling (:TbSync)]

[Here was listed a no longer valid try run, which has now been removed.]

Comment 9

[John Bieling (:TbSync)]

try: https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=a70555894ca035231f0fecae5e0202b65b1c429b

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2886757a85a9
Return the correct load status for message tabs. r=darktrojan

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e19f3cea28fb
follow-up - add files accidentially left out when unbitrotting. rs=bustage-fix

Comment 12

[John Bieling (:TbSync)]

Something went wrong here. The patch does not include

mail/components/extensions/test/browser/browser_ext_tabs_onCreated_bug1817872.js

but its in the try:
https://hg.mozilla.org/try-comm-central/diff/559f0851347f07d185f8b1fbc0137e42f29e5335/mail/components/extensions/test/browser/browser_ext_tabs_onCreated_bug1817872.js

But file

mail/components/extensions/test/browser/browser_ext_tabs_move.js

is probably wrong. Will check. Sorry. Sorry. Sorry.

Comment 13

[John Bieling (:TbSync)]

Everything seems to look good. But I have no idea what happened. The history shows, that the file was removed from the patch, but I do not know how and why:
https://phabricator.services.mozilla.com/D175276?vs=710014&id=710489#toc

I probably should have rebased before requesting checkin, is that it?

Comment 14

[Magnus Melin [:mkmelin]]

Two of the patches landing needed fixing due to a conflict in browser.ini. Since such a conflict happened I had to fix+add back commit message. I had forgot to hg add the added files. Other than that, yes something must have been included in the wrong patch or patches, but I didn't go figuring out exactly what since the end result seemed to work.