1862387 - Add support for data: and blob: urls to be used as menu icons.

Status: RESOLVED FIXED

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

Description

[Teal Dulcet [:tdulcet] (TB Council)]

Attachment: image.png

Both my Bug Opener and GIMPS Opener extensions are broken in Thunderbird 115 with this exception:

Error: Type error for parameter updateProperties (Error processing icons: Value must either: .16 must match the format "strictRelativeUrl", or be a string value) for menus.update.

The same two add-ons work as expected in Thunderbird 102, as well as all versions of Firefox.

The regression seems to be related to the use of a URL for the menu icon, for example with this code:

menus.update(`${aid}-${menuItem.name}`, {
	title: menuText,
	icons: {
		16: menuItem.icon || `${url.origin}/favicon.ico`
	},
	visible: Boolean(menuItem.url)
});

Both extensions have the <all_urls> permission, so that should not be an problem.

I ran mozregression and here was the result:

2023-11-01T02:45:55.172000: INFO : Narrowed nightly regression window from [2022-10-15, 2022-10-17] (2 days) to [2022-10-15, 2022-10-16] (1 days) (~0 steps left)
2023-11-01T02:45:55.866000: WARNING : Skipping build 4da84d529b2f: Unable to find build info using the taskcluster route 'comm.v2.comm-central.revision.4da84d529b2f6d595019e0a71142bab2454c9ef8.thunderbird.win64-opt'
2023-11-01T02:45:55.882000: WARNING : Skipping build d1cd9c1c9ac5: Unable to find build info using the taskcluster route 'comm.v2.comm-central.revision.d1cd9c1c9ac5700b3624b215f16e340e953d75ef.thunderbird.win64-opt'
2023-11-01T02:45:55.992000: WARNING : Skipping build 120942422c67: Unable to find build info using the taskcluster route 'comm.v2.comm-central.revision.120942422c67529a29ed3a563c6b4052705295d2.thunderbird.win64-opt'
2023-11-01T02:45:59.748000: INFO : Stopped

Comment 1

[John Bieling (:TbSync)]

Could you share the value of url.origin in that example?

Edit: Ah, it is a remote url.

Comment 2

[John Bieling (:TbSync)]

Hm. I find it difficult to call this an regression. I am following the spec to the letter:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/update#icons
https://webextension-api.thunderbird.net/en/stable/menus.html#menus-iconpath

This clearly states that a path must be a relative path. If a remote path worked before, then it was outside the spec. I will check back with the Firefox people, if they allowed this by accident.

Comment 3

[Rob Wu [:robwu]]

(In reply to John Bieling (:TbSync) from comment #2)

Hm. I find it difficult to call this an regression. I am following the spec to the letter:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/update#icons
https://webextension-api.thunderbird.net/en/stable/menus.html#menus-iconpath

This clearly states that a path must be a relative path. If a remote path worked before, then it was outside the spec. I will check back with the Firefox people, if they allowed this by accident.

Remote URL support is unintentional. The intent was to only permit extension URLs.

The implementation uses extension.baseURI.resolveto resolve relative URLs, but that doesn't behave as expected, as seen at bug 1795082.

Comment 4

[Teal Dulcet [:tdulcet] (TB Council)]

Thanks for the information. I would argue that this behavior should be made intentional and documented, as it is useful and very much needed. While extensions could of course manually download the remote icons, according to that official documentation on MDN, the menus API does not accept an ImageData object or a data: URI, so there would be no way to dynamically set the menu icons from the downloaded data. Even if this were supported, there should be no need for extensions to reimplement functionality that the browser (Firefox/Thunderbird) already provides.

Please consider reopening this bug and restoring this needed functionality from Thunderbird 102 and before.

Comment 5

[Teal Dulcet [:tdulcet] (TB Council)]

I just checked and Firefox does support using data:image URIs as well and it looks like several add-ons on AMO depend on this (undocumented) feature, including at least one that is recommended by Mozilla. However, I can confirm that this functionality is also broken in Thunderbird 115, as it produces the same exception as in comment 0.

Again, please consider reopening this bug and either provide a Thunderbird specific workaround to dynamically set the menu icons or restore the functionality from Thunderbird 102. I would be happy to open a new enhancement bug if needed.

Comment 6

[John Bieling (:TbSync)]

I am fine with adding support for data:image or blob urls. I just do not want to go against the specs. The best thing you can do is to open a bug at Firefox and request clarification on the supported means and an update of the documentation. I fill follow with whatever Firefox officially supports.

Comment 7

[Rob Wu [:robwu]]

Re-opening; on Firefox's side we have decided to continue support for icons that aren't packaged with the extension. Rationale is at https://bugzilla.mozilla.org/show_bug.cgi?id=1864187#c3 (plus backwards-compatibility concerns).

Comment 8

[John Bieling (:TbSync)]

Attachment: Bug 1862387 - Add support for data: and blob: urls to be used as menu icons. r=#thunderbird-reviewers

Depends on D195621

Comment 9

[Teal Dulcet [:tdulcet] (TB Council)]

Thank you John for the quick turnaround on fixing this in Thunderbird. I apologize if I am misunderstanding something, but it looks like your patch just adds support for data: and blob: URIs, but not "http(s)" URLs that Firefox also committed to supporting in Bug 1864187 comment 3. As explained above, restoring support for using HTTPS URLs for the menu icon is specifically what my two add-ons would need for Thunderbird 115.

Comment 10

[John Bieling (:TbSync)]

Support for http(s) needs changes in the API, not just in the schema. I understood the mentioned comment as such that m-c will implement mitigations, and I will then use the same.

IIRC, you do not need support for http. You can fetch the icon and dump it into a data or blob url.

const image = await fetch("https://raw.githubusercontent.com/thunderbird/sample-extensions/master/hello-world/images/internet.png")
const imageBlob = await image.blob();

// Retrieve a data: url.
const dataUrl = await new Promise(resolve => {
  var reader = new FileReader();
  reader.onload = (e) => resolve(e.target.result);
  reader.readAsDataURL(imageBlob);
})

// Retrieve a blob: url.
const blobUrl = URL.createObjectURL(imageBlob);

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a30ffafa5b45
Add support for data: and blob: urls to be used as menu icons. r=mkmelin

Comment 12

[Teal Dulcet [:tdulcet] (TB Council)]

Thanks for the information and example code. I will consider adding code that to my two extensions if Firefox does not implement those mitigations sometime soon. I believe one would need to use a data: URI so that it could be cached using the storage.local API, as my understanding is that blob: URLs would be invalid after a Firefox/Thunderbird restart.

Comment 13

[John Bieling (:TbSync)]

Yes, blob: URLs will be invalidated after a restart. But you could simply create a new blob: url, when your add-on needs it. I do not think you need to put the icons into your local storage (you do not seem to do that currently).

What you could use is a little wrapper function:

// Somewhere in the global scope
const iconBlobUrlCache = new Map();

// ...

menus.update(`${aid}-${menuItem.name}`, {
	title: menuText,
	icons: {
		16: menuItem.icon || getBlobUrl(`${url.origin}/favicon.ico`)
	},
	visible: Boolean(menuItem.url)
});

async function getBlobUrl(url) {
	if (iconBlobUrlCache.has(url)) {
		return iconBlobUrlCache.get(url);
	}
	const icon = await fetch(url);
	const iconBlob = await icon.blob();
	const iconBlobUrl = URL.createObjectURL(iconBlob);
	iconBlobUrlCache.set(url, iconBlobUrl);
	return iconBlobUrl;
}

I think the mitigation code which Firefox wants to add to the API internally, to reduce tracking thru the icon url-requests each time the menu is shown, will look something like that.

Comment 14

[Teal Dulcet [:tdulcet] (TB Council)]

OK, thanks for the information again and for the code. I was just trying to replicate the Thunderbird 102 (and Firefox) behavior, which seemed to internally cache the icons for a long time (days or weeks?). This could be seen by users since the first time one opened the menu there was always a noticeable delay for the icons to show, but on subsequent openings the icons usually appeared immediately. In addition, storing data in global variables is unfortunately of course not currently MV3 compatible, but hopefully Firefox and Thunderbird will always support persistent background pages for desktop extensions like this that require it.

Anyway, caching the icons once per session should obviously be fine for this temporary workaround while we wait for Firefox to implement those long-term mitigations. I added your suggested code to my local copies of the two respective extensions. For reference in case it is helpful to anyone else, here is the full code I used for the first extension, which hopefully properly handles all possible edge cases:

const IS_THUNDERBIRD = typeof messenger !== "undefined";
const menus = browser.menus || browser.contextMenus;

// Somewhere in the global scope
const iconBlobUrlCache = new Map();

async function getBlobUrl(url) {
	if (iconBlobUrlCache.has(url)) {
		return iconBlobUrlCache.get(url);
	}
	let iconBlobUrl = null;
	try {
		const icon = await fetch(url);
		if (icon.ok) {
			const iconBlob = await icon.blob();
			iconBlobUrl = URL.createObjectURL(iconBlob);
		}
	} catch(error) {
		console.error(error);
		return null;
	}
	iconBlobUrlCache.set(url, iconBlobUrl);
	return iconBlobUrl;
}

for (const menuItem of menuItems) {
	// ...
	let icon = menuItem.icon || `${url.origin}/favicon.ico`;
	if (IS_THUNDERBIRD) {
		icon = await getBlobUrl(icon);
	}
	menus.update(`${aid}-${menuItem.name}`, {
		title: menuText,
		icons: {
			16: icon
		},
		visible: Boolean(menuItem.url)
	});
	// ...
}

I also tested this code in the latest Thunderbird Daily and can confirm that the icons now show again as expected. I will wait until your patch is uplifted to Thunderbird 115 and I can test it there before releasing new versions of the two extensions on ATN for your review. Thanks again for all your help.

Comment 15

[John Bieling (:TbSync)]

Because this bug is not marked as fixed, it never got into the uplift queue. Changing the subject and marking it as fixed. I believe support for http(s) urls is still pending? Creating a new bug for it.

Comment 16

[John Bieling (:TbSync)]

Comment on attachment 9367239 [details]
Bug 1862387 - Add support for data: and blob: urls to be used as menu icons. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined: Add-ons, which relied on undocumented behavior, are broken (undocumented behavior is now documented)
Testing completed (on c-c, etc.): Full Beta cycle
Risk to taking this patch (and alternatives if risky): Low. Only changing the schema to allow data: and blob: urls pass parameter validation.

Comment 17

[Wayne Mery (:wsmwk)]

Comment on attachment 9367239 [details]
Bug 1862387 - Add support for data: and blob: urls to be used as menu icons. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr115

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 115.8.0:
https://hg.mozilla.org/releases/comm-esr115/rev/bc5e4db453c2