Closed Bug 1835559 Opened 1 year ago Closed 1 year ago

"Unable to check for updates due to internal error. Updates available at" missing textContent in link

Categories

(Toolkit :: Application Update, defect)

defect

Tracking

()

RESOLVED FIXED
115 Branch
Tracking Status
firefox115 --- fixed

People

(Reporter: robwu, Assigned: robwu)

References

Details

(Whiteboard: [addons-jira])

Attachments

(1 file)

On Nightly from almost 2 weeks old (115.0a1 buildid 20230516212859), I opened the "About Nightly" dialog to check for updates. It showed a new version, so I clicked on the button to update. That failed and the following message is displayed (the failure is due to bug 1811826, and not the point of this bug report, details at end), and the following message is displayed:

Unable to check for updates due to internal error. Updates available at

Note that there is nothing else at the end of the message (I expected a link). Because of this, as a user I would not be able to recover from this failed update.

When I inspect the element via the Browser Console, I see that the link href is assigned, but the textContent is missing.

Snippet: Services.wm.getEnumerator("Browser:About").getNext().document.querySelectorAll(".manualLink")[1].parentNode.innerHTML

Unable to check for updates due to internal error. Updates available at <label xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" class="manualLink text-link" is="text-link" data-l10n-name="manual-link" href="https://www.mozilla.org/en-US/firefox/nightly/?reason=manual-update"/>

This is strange, because textContent and href are assigned simultaneously at https://searchfox.org/mozilla-central/rev/f4d3fe187cf7dffa4c13b354bbde9bc47b5ccd3f/browser/base/content/aboutDialog-appUpdater.js#55-63

Steps to reproduce

This is an artificial test case based on the originally observed error (see below - bug 1811826).

STR:

  1. Run the snippet below (in the Browser Console) to intentionally break the DownloadUtils.convertByteUnits method that is relied upon by the updater.
  2. Open the "About Nightly" dialog and check for updates.
  3. If there are no updates, close the dialog and wait one day (or install an old version of Firefox and restart from step 1).
  4. Click on the "Update to 115.0a1 (2023-05-27)" button to update to the latest version.
  5. Look at the error in the About dialog that is shown instead of the button.
  6. Optional: run the snippet again to restore the internal.

Expected:

  • At step 5, I should see a link at the end of the message.

Actual:

  • At step 5, there is no link at the end of the message.

The snippet:

DownloadUtils = ChromeUtils.import('resource://gre/modules/DownloadUtils.jsm').DownloadUtils;
if (!DownloadUtils.cbu) {
  console.log("break");
  DownloadUtils.cbu = DownloadUtils.convertByteUnits;
  DownloadUtils.convertByteUnits = () => {throw new Error("Intentional error from DownloadUtils.convertByteUnits")}
} else {
  console.log("unbreak");
  DownloadUtils.convertByteUnits=DownloadUtils.cbu;
  DownloadUtils.cbu=null
}

Error from bug 1811826

I figured that bug 1811826 is triggered, because of the following stack trace when I click on the button in the about dialog to update. I verified that bug 1811826 is the cause, because accessing Services.intl throws NS_ERROR_FAILURE. For completeness, the stack trace is below.

NS_ERROR_FAILURE: 
    getLocaleNumberFormat resource://gre/modules/DownloadUtils.sys.mjs:61
    DU_convertByteUnits resource://gre/modules/DownloadUtils.sys.mjs:493
    DU_getTransferTotal resource://gre/modules/DownloadUtils.sys.mjs:216
    _onAppUpdateStatus chrome://browser/content/aboutDialog-appUpdater.js:116
    _appUpdateListener chrome://browser/content/aboutDialog-appUpdater.js:42
    #setStatus resource://gre/modules/AppUpdater.sys.mjs:727
    #downloadUpdate resource://gre/modules/AppUpdater.sys.mjs:456
    check resource://gre/modules/AppUpdater.sys.mjs:405
AppUpdater.sys.mjs:171:17
    #onException resource://gre/modules/AppUpdater.sys.mjs:171
    check resource://gre/modules/AppUpdater.sys.mjs:407

I added a unit test in bug 1835561, but skipped the test for the internalError element (in an if) because of this bug. To run the test, do:

./mach test toolkit/mozapps/update/tests/browser/browser_aboutDialog_AppUpdater_stop_internal_error.js

While working on that, I noticed that if I delete the update-manual message (i.e. revert the fix from bug 1835561), then the test passes.
Could it be that the presence of <label data-l10n-name="manual-link"/> somehow causes the second link to lose its text content? EDIT: I renamed manual-link to manual-link-2 and it did not fix it.

On the Nightly where I reproduced this bug, the update-manual message was already missing (because bug 1820654 deleted it). So this might be unrelated or test-only.


When I don't skip the check from the patch to bug 1835561, then the test fails with:

toolkit/mozapps/update/tests/browser/browser_aboutDialog_AppUpdater_stop_internal_error.js
  FAIL The panel's link should have non-empty textContent, got "" -
Stack trace:
chrome://mochikit/content/browser-test.js:test_ok:1582
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/head.js:assertNonEmptyText:836
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/head.js:runAboutDialogUpdateTest/processAboutDialogStep/<:838
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/head.js:processAboutDialogStep:866
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/head.js:runAboutDialogUpdateTest/<:932
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/head.js:runAboutDialogUpdateTest:934
chrome://mochitests/content/browser/toolkit/mozapps/update/tests/browser/browser_aboutDialog_AppUpdater_stop_internal_error.js:aboutDialog_AppUpdater_stop_internal_error:34

I think that the translation is happening after Fluent did its thing. This is like bug 1738056, and affects the following messages in browser/aboutDialog.ftl:

  • update-downloading
  • update-downloading-message
  • update-manual (restored in bug 1835561 after mistakenly deleted in bug 1820654).
  • aboutdialog-update-manual

I looked for similar occurrences of the issue, and found one, with an existing bug (bug 1446389):

Search queries:

When an element is re-translated, the textContent is currently dropped.
To avoid that, make it an explicit part of the message.

This requires changing the l10n message ID.
Ideally, the message ID wouldn't change, but that is not possible until
the content can be preserved as a feature, i.e.
https://github.com/projectfluent/fluent.js/issues/169

Assignee: nobody → rob
Status: NEW → ASSIGNED

Your changes in bug 1835561 caused me to be backed out in bug 1831259.

 0:17.98 FAIL The panel's link should have non-empty textContent, got "" - "" == true - got "", expected true (operator ==)

Given that I am not touching the updater code, and that awaiting the panes being loaded (or awaiting full fluent translation using both l10n.ready and hasPendingL10nMutations) does not help, my interpretation is that the current code is racy and that the patch in this bug should fix things. Is that accurate?

Blocks: 1831259

(In reply to :Gijs (he/him) from comment #4)

Your changes in bug 1835561 caused me to be backed out in bug 1831259.

 0:17.98 FAIL The panel's link should have non-empty textContent, got "" - "" == true - got "", expected true (operator ==)

Given that I am not touching the updater code, and that awaiting the panes being loaded (or awaiting full fluent translation using both l10n.ready and hasPendingL10nMutations) does not help, my interpretation is that the current code is racy and that the patch in this bug should fix things. Is that accurate?

Yes. I put an explicit translateFragment call at https://searchfox.org/mozilla-central/rev/daedd554ae8a2c7f420ad77311134c8c298ba318/toolkit/mozapps/update/tests/browser/head.js#846 because otherwise the element would not be translated, but the preceding lines should also be covered by an explicit wait for the translation to complete. The current patch in this bug does that.

Whiteboard: [addons-jira]
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/81758aba6ab3 Ensure that text content survives translation r=application-update-reviewers,flod,fluent-reviewers,nalexander,settings-reviewers,Gijs

I cannot reproduce locally, but I suspect that it may be a race due to the l10n not having finished yet. I've added an explicit check + wait for l10n readiness in the test and am going to reland the patch.

Flags: needinfo?(rob)
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/0c9bda2daacf Ensure that text content survives translation r=application-update-reviewers,flod,fluent-reviewers,nalexander,settings-reviewers,Gijs
Regressions: 1836162
Regressions: 1836169
Regressions: 1836190
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch
Regressions: 1836504
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: