Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ attached to the most recent release about once a day and upgrades in place;
there is nothing to re-download by hand. Open a compose window and the button
appears in the format toolbar.

Code blocks go into **HTML compose windows only**. Thunderbird hides the format
toolbar in a plain-text composer, and the button lives in that toolbar, so there
is nothing to click - and the add-on keeps its context-menu item out of a
plain-text composer for the same reason, rather than offering an insert it
cannot carry out. Which editor a composer gets is the account's own setting,
and holding Shift as you start a message opens the other one for that message.

## Building the archive

You do not need this to use the add-on - releases are built by CI from a tag.
Expand Down Expand Up @@ -191,14 +198,14 @@ console. That is `tests/thunderbird/insertion.test.js`, and every assertion in
it used to be a line on the release checklist.

The harness itself is `tests/thunderbird/harness/`, and its interface is
documented in `tests/thunderbird/harness/index.js` - including four limits
found while building it, which are worth reading before writing a test that
runs into them. The popup's document cannot be read from outside; the popup has
documented in `tests/thunderbird/harness/index.js` - including four things
worth reading before writing a test that runs into them. The popup's document cannot be read from outside; the popup has
to be handed the keyboard before it hears anything, and a test that forgets can
pass while asserting nothing; a letter-key shortcut cannot be delivered to
Thunderbird 128 by synthesised input; and the popup cannot be opened in a
plain-text composer at all, which is a defect in the add-on rather than a limit
of the harness.
plain-text composer at all, which is the add-on's scope rather than a limit of
the harness - it offers no route in there, so a test of the plain-text insert
has to reach past that by hand.

What is still checked by hand is anything that is a claim about Thunderbird
rather than about this project's own logic; that list is
Expand Down
10 changes: 7 additions & 3 deletions docs/release-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ unless another file is named:
- `&`, `<`, `>` and `"` in the source reaching the message as those characters.
- The popup closing when the insert lands.
- A plain-text composer receiving the source as text with no markup in it.
The test unhides the format toolbar to get there, because as things stand the
popup cannot be opened in a plain-text composer at all - issue #12. What is
covered is the insert; what is broken is reaching it.
The test unhides the format toolbar to get there, because the add-on offers a
plain-text composer no route to the popup and is not meant to: it inserts into
HTML mail. What is covered is the insert; reaching it is not something a user
can do.
- The context-menu item being in an HTML composer's body menu and not in a
plain-text composer's, which is the one route that could have offered an
insert nothing could carry out.
- The shortcut inserting exactly what the button inserts, and the manifest's
`Ctrl+Shift+C` having become the key element Thunderbird derives from it.
**Delivering that key press is not covered** - see the first item under
Expand Down
101 changes: 98 additions & 3 deletions src/background/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import { TAKE_PENDING_SELECTION } from "../messaging/take-pending-selection.js";
*
* 1. This file's scope is re-executed every time an event wakes the page, so
* top-level work must be safe to repeat.
* 2. Anything held in module scope is lost when the page is suspended. The one
* piece of state here - the parked selection - is written and read within a
* single user gesture, which is the only lifetime it can rely on.
* 2. Anything held in module scope is lost when the page is suspended. Both
* pieces of state here - the parked selection and the menu that is open -
* are written and read within a single user gesture, which is the only
* lifetime either can rely on.
*/

/**
Expand All @@ -36,6 +37,27 @@ const MENU_ID = "thundercode-insert-code-block";
*/
const pendingSelections = new Map();

/**
* How many menus have opened since this page was woken, which is only ever
* read as a way of telling one of them from the next.
*/
let menusOpened = 0;

/**
* Which of those menus is on screen, and zero when none is.
*
* Deciding whether the item belongs in a menu means asking the composer what
* format it is in, and the menu is already drawn by the time the answer comes
* back. Without this, an answer that arrives late would set the item's
* visibility for whichever menu is open by then - so a menu the add-on has
* nothing to say about would be handed the previous one's answer.
*
* Module scope, so both of these are lost when the event page is suspended.
* That costs nothing: a menu cannot outlive the page that is being woken to
* answer it.
*/
let menuOnScreen = 0;

/**
* Creating the item at file scope means it exists as soon as the page runs,
* which on an event page is also every time it is woken. The duplicate-id error
Expand All @@ -53,6 +75,13 @@ function createMenu() {
// would also match selections in the message reader and put the item in
// menus that have no composer to insert into.
contexts: ["compose_body"],

// Hidden until a composer has been asked what format it is in, which is
// what `menus.onShown` below does. Created visible instead, the item
// would be in the menu for as long as that answer takes to arrive - and
// the composer where a false offer costs something is precisely the one
// where the answer is "plain text".
visible: false,
},
() => void browser.runtime.lastError,
);
Expand All @@ -65,11 +94,77 @@ createMenu();
// before the user opens their first composer.
browser.runtime.onStartup.addListener(createMenu);

/**
* Whether this composer is one the add-on can put a code block into.
*
* The block goes in as markup, and the popup is reached through a button in
* the format toolbar - which Thunderbird hides in a plain-text composer, there
* being no formatting to offer. So a plain-text composer is a composer this
* add-on has nothing to do in, and the honest thing is to offer it nothing.
*
* A tab that cannot be asked - it has closed, or was never a composer -
* answers the same way. An offer this add-on could not check is one it cannot
* promise to keep.
*/
async function canTakeACodeBlock(tabId) {
try {
const { isPlainText } = await browser.compose.getComposeDetails(tabId);
return !isPlainText;
} catch {
return false;
}
}

/**
* The item's visibility, decided per menu rather than once at creation.
*
* `onShown` fires for every menu this add-on could have an item in, including
* menus it has nothing in, so the context is checked before anything is
* touched: the item is one piece of state shared by every window, and an
* update made on behalf of a menu it is not in would be waiting in the next
* menu it is.
*
* `refresh` is the half without which none of this is visible. The menu is
* already on screen when this runs, so an item whose visibility has just
* changed keeps being drawn the old way until the menu is rebuilt.
*/
browser.menus.onShown.addListener(async (info, tab) => {
if (!info.contexts.includes("compose_body") || !tab) {
return;
}

const menu = ++menusOpened;
menuOnScreen = menu;

const visible = await canTakeACodeBlock(tab.id);
if (menuOnScreen !== menu) {
return;
}

await browser.menus.update(MENU_ID, { visible });
await browser.menus.refresh();
});

browser.menus.onHidden.addListener(() => {
menuOnScreen = 0;
});

browser.menus.onClicked.addListener(async (info, tab) => {
if (info.menuItemId !== MENU_ID || !tab) {
return;
}

// Asked again rather than taken on trust from the item being visible: that
// visibility is state Thunderbird holds between one menu and the next, so a
// click can come from a menu drawn before `onShown` above had its say.
// Returning before anything is parked is the whole of what this has to get
// right - a selection left behind here would surface in the next popup this
// tab opens by some other route.
if (!(await canTakeACodeBlock(tab.id))) {
pendingSelections.delete(tab.id);
return;
}

// Plain text extracted from HTML by Thunderbird, so its indentation may
// already be damaged before this extension sees it - the popup treats it as
// a convenience, not as the source of truth. It is present only because the
Expand Down
Loading