1860304 - Background page fails to load if background.service_worker and background.scripts are both present

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

When manifest.json's background declaration contains both service_worker and an event page, the extension loads but the background page fails to start up.

The expected cross-browser behavior (as established in https://github.com/w3c/webextensions/issues/282) is to ignore scripts if service_worker is supported, but to use scripts otherwise (WECG proposal / implemented by Safari). Chrome currently fails to load the extension, but will reduce it to a non-fatal warning in crbug.com/1418934.

We currently don't support service_worker (bug 1573659). And as I explained in https://bugzilla.mozilla.org/show_bug.cgi?id=1775618#c1, there is already a bug that prevents the Service worker from loading if scripts and service_worker are both present. Fixing that was not a high priority because we haven't turned on support for service workers.

Today, I saw a report (https://github.com/w3c/webextensions/issues/282#issuecomment-1773034430) that stated that the background page fails to load if both are present. Upon investigation, it turns out that this is caused by a combination of bugs:

• bug 1831417: unrecognized properties still exist in unvalidated form.
• The logic that selects BackgroundWorker or BackgroundPage selects a non-functional service worker implementation because manifest.background.service_worker is present, at https://searchfox.org/mozilla-central/rev/9f6a9e601b2ab9ad1a3877691ea17c66c1fd8867/toolkit/components/extensions/parent/ext-backgroundPage.js#678-680

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1609920

Comment 2

[carlos]

Thanks for opening this! As far as I know having an unrecognised property under background would not prevent the background page/scripts to load. Unless it's the service_worker. Is it known when an extension with both scripts and service_worker started becoming unsupported?

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1609920

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1860304 - Ignore service_worker property when off by pref

When a background page and service_worker are specified simultaneously,
load the background page instead of neither (i.e. the bug).

This patch only fixes the issue when
extensions.backgroundServiceWorker.enabled is false; When true, the
service worker is expected to load but doesn't due to
https://bugzilla.mozilla.org/show_bug.cgi?id=1775618#c1 . That requires
a different fix and is therefore saved for a follow-up.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf96c386d8c7
Ignore service_worker property when off by pref r=rpl

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/bf96c386d8c7

Comment 7

[Rob Wu [:robwu]]

Needs MDN release note entry.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:robwu, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox120 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[rbloor]

Release note available for review in "Background page load fail bug fix release note" #30097

Comment 10

[Rob Wu [:robwu]]

Attachment: Bug 1860304 - Ignore service_worker property when off by pref r=rpl

When a background page and service_worker are specified simultaneously,
load the background page instead of neither (i.e. the bug).

This patch only fixes the issue when
extensions.backgroundServiceWorker.enabled is false; When true, the
service worker is expected to load but doesn't due to
https://bugzilla.mozilla.org/show_bug.cgi?id=1775618#c1 . That requires
a different fix and is therefore saved for a follow-up.

Original Revision: https://phabricator.services.mozilla.com/D192081

Comment 11

[Phabricator Automation]

esr115 Uplift Approval Request

• User impact if declined: ESR115 users cannot install extensions that have been developed with cross-browser compatibility in mind. Since ESR115 is the last supported Firefox version on Windows 7/8, this could lower the number of available add-ons to them in the longer term.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Create a directory with manifest.json and background.js from https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/background#browser_support . Then try to load it at about:debugging and confirm that the test extension works as documented (a new tab opens)
• Risk associated with taking this patch: Low
• Explanation of risk level: Fixes very specific check, without side effects
• String changes made/needed: no
• Is Android affected?: yes

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/d962391def52

Comment 13

[Alex Cornestean]

Verified as Fixed. Tested on the latest ESR 115.11.0 (https://treeherder.mozilla.org/jobs?repo=mozilla-esr115&searchStr=build-signing%2Cshippable&fromchange=0b1b7b40dce62a5b81da7743f0d78dcf3ff3cead&selectedTaskRun=PvYBK5cXQsybruXrgiJpdQ.0) under Windows 10 x64 and Ubuntu 22.04 LTS.

Upon loading the test extension created as per the specifications mentioned in https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/background#browser_support, a new tab opens with the following URL - http://example.com/firstrun.html, confirming the fix.