Copying multiple tabs in rich-text HTML format sometimes/randomly creates a blank small Firefox window and does not copy (race condition?)

[nekohayo]

Steps to reproduce

1. Create the "HTML Link (Rich Text)" template as per the readme file's examples: %RT%<a title="%TITLE_HTML%" href="%URL_HTML%">%TITLE_HTML%</a>
2. Select multiple tabs
3. Right-click > "Copy HTML Link (Rich Text)"

Result

Sometimes it works, sometimes it doesn't and I get a tiny empty floating Firefox window (and when that happens, you know the copy didn't work). Here is a video that demonstrates the issue.

Firefox.copy-selected-tabs-to-clipboard.extension.randomly.fails.to.copy.rich-text.HTML.items.mp4

When it happens, the wanted information is not actually copied to the clipboard (or at least, my email client cannot paste it, nor can LibreOffice Writer).

Strangely enough, the other types of copies (ex: "HTML Link", "Markdown Link List") seem to work fine "every time". Just not the rich-text HTML one.

Environment

• Platform (OS): Fedora Linux, under GNOME and Xorg/X11
• Version of Firefox: 104
• Version (or revision) of Multiple Tab Handler: 1.4.5 (2022.3.16)

[piroor]

Could you try collecting error message in the debug console? You can see it with steps:

1. Type about:debugging into the location bar and hit the Enter key.
2. Choose "This Firefox" at the left pane.
3. Find "Copy Selected Tabs to Clipboard" from the list and click the Inspect button.
4. Then a debugger tab is opened. Click "Console" to see error messages.
5. Click the dustbox icon to clear existing messages.
6. Try to reproduce the problem you saw.
7. After the problem appears, go back to the debugger tab and collect messages reported in the console.

[nekohayo]

This is the message I got from the console, it appeared maybe 1 second after I focused back the tab containing the console after triggering the bug and closing the little "ghost" empty window.

Uncaught (in promise) Error: An unexpected error occurred undefined
    onClick moz-extension://8112725a-6705-4dee-8045-59972c070321/background/context-menu.js:267
    AsyncFunctionThrow self-hosted:673
    (Async: async)
    apply self-hosted:2252
    applySafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:622
    applySafe resource://gre/modules/ExtensionCommon.jsm:605
    sync resource://gre/modules/ExtensionCommon.jsm:2496
    listener chrome://browser/content/child/ext-menus.js:280
    withHandlingUserInput resource://gre/modules/ExtensionCommon.jsm:113
    listener chrome://browser/content/child/ext-menus.js:279
    apply self-hosted:2252
    applySafeWithoutClone resource://gre/modules/ExtensionCommon.jsm:622
    fire resource://gre/modules/ExtensionChild.jsm:829
    recvRunListener resource://gre/modules/ExtensionChild.jsm:833
    recvRunListener self-hosted:1157
    _recv resource://gre/modules/ConduitsChild.jsm:82
    receiveMessage resource://gre/modules/ConduitsChild.jsm:188

[nekohayo]

I don't know if this is also relevant, but after I copied the above, for some unknown reason a new tab opened up with these contents on a plain black-on-white page, too:

/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";

/**
 * This @file implements the child side of Conduits, an abstraction over
 * Fission IPC for extension API subject.  See {@link ConduitsParent.jsm}
 * for more details about the overall design.
 *
 * @typedef {object} MessageData
 * @prop {ConduitID} [target]
 * @prop {ConduitID} [sender]
 * @prop {boolean} query
 * @prop {object} arg
 */

const EXPORTED_SYMBOLS = [
  "BaseConduit",
  "ConduitsChild",
  "ProcessConduitsChild",
];

/**
 * Base class for both child (Point) and parent (Broadcast) side of conduits,
 * handles setting up send/receive method stubs.
 */
class BaseConduit {
  /**
   * @param {object} subject
   * @param {ConduitAddress} address
   */
  constructor(subject, address) {
    this.subject = subject;
    this.address = address;
    this.id = address.id;

    for (let name of address.send || []) {
      this[`send${name}`] = this._send.bind(this, name, false);
    }
    for (let name of address.query || []) {
      this[`query${name}`] = this._send.bind(this, name, true);
    }

    this.recv = new Map();
    for (let name of address.recv || []) {
      let method = this.subject[`recv${name}`];
      if (!method) {
        throw new Error(`recv${name} not found for conduit ${this.id}`);
      }
      this.recv.set(name, method.bind(this.subject));
    }
  }

  /**
   * Internal, partially @abstract, uses the actor to send the message/query.
   * @param {string} method
   * @param {boolean} query Flag indicating a response is expected.
   * @param {JSWindowActor} actor
   * @param {MessageData} data
   * @returns {Promise?}
   */
  _send(method, query, actor, data) {
    if (query) {
      return actor.sendQuery(method, data);
    }
    actor.sendAsyncMessage(method, data);
  }

  /**
   * Internal, calls the specific recvX method based on the message.
   * @param {string} name Message/method name.
   * @param {object} arg  Message data, the one and only method argument.
   * @param {object} meta Metadata about the method call.
   */
  async _recv(name, arg, meta) {
    let method = this.recv.get(name);
    if (!method) {
      throw new Error(`recv${name} not found for conduit ${this.id}`);
    }
    try {
      return await method(arg, meta);
    } catch (e) {
      if (meta.query) {
        return Promise.reject(e);
      }
      Cu.reportError(e);
    }
  }
}

/**
 * Child side conduit, can only send/receive point-to-point messages via the
 * one specific ConduitsChild actor.
 */
class PointConduit extends BaseConduit {
  constructor(subject, address, actor) {
    super(subject, address);
    this.actor = actor;
    this.actor.sendAsyncMessage("ConduitOpened", { arg: address });
  }

  /**
   * Internal, sends messages via the actor, used by sendX stubs.
   * @param {string} method
   * @param {boolean} query
   * @param {object?} arg
   * @returns {Promise?}
   */
  _send(method, query, arg = {}) {
    if (!this.actor) {
      throw new Error(`send${method} on closed conduit ${this.id}`);
    }
    let sender = this.id;
    return super._send(method, query, this.actor, { arg, query, sender });
  }

  /**
   * Closes the conduit from further IPC, notifies the parent side by default.
   * @param {boolean} silent
   */
  close(silent = false) {
    let { actor } = this;
    if (actor) {
      this.actor = null;
      actor.conduits.delete(this.id);
      if (!silent) {
        // Catch any exceptions that can occur if the conduit is closed while
        // the actor is being destroyed due to the containing browser being closed.
        // This should be treated as if the silent flag was passed.
        try {
          actor.sendAsyncMessage("ConduitClosed", { sender: this.id });
        } catch (ex) {}
      }
    }
    this.closeCallback?.();
    this.closeCallback = null;
  }

  /**
   * Set the callback to be called when the conduit is closed.
   * @param {function} callback
   */
  setCloseCallback(callback) {
    this.closeCallback = callback;
  }
}

/**
 * Implements the child side of the Conduits actor, manages conduit lifetimes.
 */
class ConduitsChild extends JSWindowActorChild {
  constructor() {
    super();
    this.conduits = new Map();
  }

  /**
   * Public entry point a child-side subject uses to open a conduit.
   * @param {object} subject
   * @param {ConduitAddress} address
   * @returns {PointConduit}
   */
  openConduit(subject, address) {
    let conduit = new PointConduit(subject, address, this);
    this.conduits.set(conduit.id, conduit);
    return conduit;
  }

  /**
   * JSWindowActor method, routes the message to the target subject.
   * @param {string} name
   * @param {MessageData|MessageData[]} data
   * @returns {Promise?}
   */
  receiveMessage({ name, data }) {
    // Batch of webRequest events, run each and return results, ignoring errors.
    if (Array.isArray(data)) {
      let run = data => this.receiveMessage({ name, data });
      return Promise.all(data.map(data => run(data).catch(Cu.reportError)));
    }

    let { target, arg, query, sender } = data;
    let conduit = this.conduits.get(target);
    if (!conduit) {
      throw new Error(`${name} for closed conduit ${target}: ${uneval(arg)}`);
    }
    return conduit._recv(name, arg, { sender, query, actor: this });
  }

  /**
   * JSWindowActor method, ensure cleanup.
   */
  didDestroy() {
    for (let conduit of this.conduits.values()) {
      conduit.close(true);
    }
    this.conduits.clear();
  }
}

/**
 * Child side of the Conduits process actor.  Same code as JSWindowActor.
 */
class ProcessConduitsChild extends JSProcessActorChild {
  constructor() {
    super();
    this.conduits = new Map();
  }

  openConduit = ConduitsChild.prototype.openConduit;
  receiveMessage = ConduitsChild.prototype.receiveMessage;
  willDestroy = ConduitsChild.prototype.willDestroy;
  didDestroy = ConduitsChild.prototype.didDestroy;
}

[piroor]

• The error log Copying multiple tabs in rich-text HTML format sometimes/randomly creates a blank small Firefox window and does not copy (race condition?) #35 (comment) says that Firefox failed to call browser.tabs.highlight() after the context menu command was processed. It looks not related to the ghost window problem...
• The source Copying multiple tabs in rich-text HTML format sometimes/randomly creates a blank small Firefox window and does not copy (race condition?) #35 (comment) is a part of Firefox itself. Sadly it also doesn't related to this problem.

The popup type window can trigger the problem, so I've introduced some changes to copy rich text more safely with the commit 36483cd.

1. This addon tries to call navigator.clipboard.write() at first. It is the best and safest way.
2. If it failed, this tries to open a temporary tab in an existing browser window, and tries to copy the data with its content page.
3. If it failed (ex. couldn't open a new tab in the window), this tries to open a temporary window, and tries to copy the data with its content page.
4. If it failed (ex. couldn't open a new window), this gives up to copy a rich text data and just copies a plain-text data with navigator.clipboard.writeText().

Could you try a development build https://github.com/piroor/copy-selected-tabs-to-clipboard/actions/runs/3001470431#artifacts with about:debugging ? You can load the unsigned XPI with the "Load Temporary Add-on..." button.