Affected Version:
Latest Nightly 76.0a1 (2020-03-30)

Affected Platforms:
Windows 10 x64
Windows 7 x32
Windows 10 win64-aarch64

Steps to reproduce:
STR1:

1. Have an older Nightly (FX73) installed in the default location
2. Open the old Nightly and install some addons and themes, close it after that
3. Download the stub installer for the Latest Nightly and run it with default settings ("pave-over method")

STR2:

1. Install latest Nightly with stub installer
2. Open Nightly, add some addons and themes
3. Uninstall this Nightly
4. Launch the latest Nightly stub installer and go through the process with default settings

Expected Results:
When the import wizards dialog appears, the "< Back/Finish/Cancel" buttons should be correctly displayed

Actual Results:
The button labels are missing and the dialog closes by itself after 2 seconds

Regression-Range:
This is a regression, latest Beta75.0b9 and Release 74 are not affected.
Can't mozregress this issue due to the nature of it..

[Tracking Requested - why for this release]: Bad UI regression

I'm guessing this was caused by bug 1608200. There was a warning about this case in bug 1608200 comment 1 but I'm not sure anyone tested that case with the patch.

A simple --migration is sufficient to reproduce this (at least on macOS) so mozregression should be able to confirm the range.

I can reproduce the issue on Nightly76.0a1 Windows10 and Ubuntu18.04

Regression window on Ubuntu:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a45ebe808a75bc2a2354f7d544940be1cf51a1a2&tochange=20b1cf25be1bf0f6afce979a3276712a0182ff7e

Hey gandalf, any idea why the Fluent strings aren't available in this case? We had presumed via bug 1608165 comment 4 that this was safe to do - did we do it wrong?

Thanks Alice!

They should be available.

I'd need STR based on comment 2 to reproduce it. Is it possible to reproduce on Linux or do I need to build on mac?

Interestingly,
After failing once, the problem is gone the second time around.

STR:
Start Nightly with New profile
Alt -> File -> Import from Another Browser... , then the buttons are empty.
Close the dialog.
Alt -> File -> Import from Another Browser..., then it is working without problem.

Yep, I can easily reproduce using ./mach run --migration.

(In reply to Matthew N. [:MattN] (PM me if request are blocking you) from comment #1)

[Tracking Requested - why for this release]: Bad UI regression

I'm guessing this was caused by bug 1608200. There was a warning about this case in bug 1608200 comment 1 but I'm not sure anyone tested that case with the patch.

I tested it with the other patch (create profile wizard) and it worked there, so I assumed it'd work here. This is also after already finding other race conditions to do with Custom Elements and shadow DOM in a separate bug... :-\

Anyway, it doesn't make a lot of sense that this case is worse, as this case has more of a profile than the other one...

Renewing Zibi's ni per comment #8.

The issue is that the localization is hooked on DOMContentLoaded which fires before connectedCallback here: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/wizard.js#528-535

Instead, this code should just:

if (document.l10n) {
  document.l10n.connectRoot(this);
  document.l10n.translateFragment(this);
}

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #10)

The issue is that the localization is hooked on DOMContentLoaded which fires before connectedCallback here: https://searchfox.org/mozilla-central/source/toolkit/content/widgets/wizard.js#528-535

Instead, this code should just:

if (document.l10n) {
  document.l10n.connectRoot(this);
  document.l10n.translateFragment(this);
}

This doesn't work because document.l10n is null.

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

This doesn't work because document.l10n is null.

More specifically, if using document.l10n directly, the buttons in the wizard are localized correctly the first time, but not any of the subsequent times you bring up the migration wizard, and TypeError: Can't access property "connectRoot", document.l10n is null shows up in the console.

This doesn't work because document.l10n is null.

Hmm, it shouldn't be. I just tested it and it works on my machine. In particular, the reason I'd expect it to work is that there's insertFTLIfNeeded which should call https://searchfox.org/mozilla-central/source/dom/base/Document.cpp#3883 which should synchronously enable document.l10n even if it was not constructed before.

I tested again and this change:

--- a/toolkit/content/widgets/wizard.js
+++ b/toolkit/content/widgets/wizard.js
@@ -520,24 +520,20 @@
   class MozWizardButtons extends MozXULElement {
     connectedCallback() {
       this._wizard = this.getRootNode().host;
 
       this.textContent = "";
       this.appendChild(MozXULElement.parseXULToFragment(this._markup));
 
       MozXULElement.insertFTLIfNeeded("toolkit/global/wizard.ftl");
-      document.addEventListener(
-        "DOMContentLoaded",
-        e => {
-          document.l10n.connectRoot(this);
-          document.l10n.translateRoots();
-        },
-        { once: true }
-      );
+      if (document.l10n) {
+        document.l10n.connectRoot(this);
+        document.l10n.translateFragment(this);
+      }
 
       this._wizardButtonDeck = this.querySelector(".wizard-next-deck");
 
       this.initializeAttributeInheritance();
 
       const listeners = [
         ["back", () => this._wizard.rewind()],
         ["next", () => this._wizard.advance()],

with the provided STR (./mach run --migration) show a wizard with localized Cancel and Next buttons on my linux mozilla-central build from today.

Are those STR with this patch still reproduce the bug for you?

More specifically, if using document.l10n directly, the buttons in the wizard are localized correctly the first time, but not any of the subsequent times you bring up the migration wizard, and TypeError: Can't access property "connectRoot", document.l10n is null shows up in the console.

Can you provide STR for that? The STR I'm using ./mach run --migration is not applicable for the scenario you're describing.

I'm also confused about the error because if (document.l10n) should return false if document.l10n is null and therefore not execute the connectRoot.

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #14)

More specifically, if using document.l10n directly, the buttons in the wizard are localized correctly the first time, but not any of the subsequent times you bring up the migration wizard, and TypeError: Can't access property "connectRoot", document.l10n is null shows up in the console.

Can you provide STR for that? The STR I'm using ./mach run --migration is not applicable for the scenario you're describing.

I'm also confused about the error because if (document.l10n) should return false if document.l10n is null and therefore not execute the connectRoot.

Just open the browser normally and then use "Import from another browser" in the "File" menu.

If I add the if (document.l10n) condition, the error obviously doesn't show up anymore, but neither do the localized strings. I also tried swapping translateRoots for translateFragment(this) as your comment #13 does, that makes no difference.

I'll investigate.

Ok, I know what's going on. Bug 1617669 which just landed makes it easier to debug, and it would be nice to land bug 1621674 first to make it easier to fix.

Basically, since the document has no Fluent in XUL file, when we add FTL file dynamically, it gets in a weird mode where it adds itself to blocking, but never resolves which unfortunately leads to DOMContentLoaded never being fired.

If this has to land ASAP, the easiest fix is to trick the system temporarily by adding:

<linkset>
  <html:link rel="localization" href="branding/brand.ftl"/>
</linkset>

to the document before any WC gets initialized.

The proper fix is going to help get document.l10n available as soon as the first FTL link gets added.

Stealing this per matrix discussion to fix the dialogs in time for 76 branching; Zibi will fix fluent/doml10n in a separate bug.

Backed out changeset 9ce55318603b (bug 1625938) for causing DOMLocalization.cpp asertion failures
https://hg.mozilla.org/integration/autoland/rev/1d96f4423d1622370f0dac09d475f8ea76b144a6

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Ctest-macosx1014-64%2Fdebug-mochitest-browser-chrome-e10s-12%2Cm%28bc12%29&fromchange=9ce55318603bf50f1a05b7033ff6a6a870e242af&tochange=1d96f4423d1622370f0dac09d475f8ea76b144a6

(In reply to Andrei Ciure[:aciure] from comment #21)

Backed out changeset 9ce55318603b (bug 1625938) for causing DOMLocalization.cpp asertion failures
https://hg.mozilla.org/integration/autoland/rev/1d96f4423d1622370f0dac09d475f8ea76b144a6

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=os%2Cx%2C10.14%2Cdebug%2Cmochitests%2Ctest-macosx1014-64%2Fdebug-mochitest-browser-chrome-e10s-12%2Cm%28bc12%29&fromchange=9ce55318603bf50f1a05b7033ff6a6a870e242af&tochange=1d96f4423d1622370f0dac09d475f8ea76b144a6

GECKO(1759) | Assertion failure: root != &aNode && !root->Contains(&aNode) && !aNode.Contains(root) (Cannot add a root that overlaps with existing root.), at /builds/worker/checkouts/gecko/dom/l10n/DOMLocalization.cpp:99

I guess the code should be doing connectRoot(this.shadowRoot) instead. Testing locally before I repush.

smaug - are we running into the same problem as with WC? Is Node::Contains respecting boundaries?

If I have a DOMLocalization connected to document's body, and then I'll connect it to a shadowRoot within this document, will it trigger: https://searchfox.org/mozilla-central/source/dom/l10n/DOMLocalization.cpp#93-101 ?

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #23)

smaug - are we running into the same problem as with WC? Is Node::Contains respecting boundaries?

If I have a DOMLocalization connected to document's body, and then I'll connect it to a shadowRoot within this document, will it trigger: https://searchfox.org/mozilla-central/source/dom/l10n/DOMLocalization.cpp#93-101 ?

I think this was just a problem with the JS code - this for the wizard button component is not the shadowroot, and in fact there isn't even a shadow root for this element, so it overlaps with the shadowroot from the containing component (the wizard).

https://hg.mozilla.org/mozilla-central/rev/87b08f234fe8

Verified-fixed on latest Nightly 76.0a1 (Build ID: 20200402095145) on Windows 7/10 x64, MacOS 10.13 and Ubuntu 18.04 - with the steps form Comment 15.
Also verified stub installer scenario for Windows 10 x64.

node.contains does respect shadow boundaries.

Also reverified fixed as part of PI 551; tested with Nightly 77 across platforms (Windows 10, macOS 10.15 and Ubuntu 18.04).