Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Firefox 86 breaks Defered Bootstrap (window.name empty) #17117

Open
1 of 4 tasks
zeha opened this issue Mar 2, 2021 · 10 comments
Open
1 of 4 tasks

Firefox 86 breaks Defered Bootstrap (window.name empty) #17117

zeha opened this issue Mar 2, 2021 · 10 comments

Comments

@zeha
Copy link

zeha commented Mar 2, 2021

I'm submitting a ...

  • regression from 1.7.0
  • security issue
  • issue caused by a new browser version
  • other

Current behavior:

Firefox 86 resets window.name to an empty string. This breaks the Deferred Bootstrap(*) feature used by Protractor.

(*): https://docs.angularjs.org/guide/bootstrap#deferred-bootstrap

I've filed this here, because even if Protractor could change something, some other way of triggering Deferred Bootstrap is probably needed.

Expected / new behavior:

Test runners like Protractor can successfully use Deferred Bootstrap with Firefox.

Minimal reproduction of the problem with instructions:

Should happen with any minimal project and a minimal protractor setup.

AngularJS version: 1.8.2

Browser: Firefox 86

Anything else:

Protractor reports:

[17:04:38] E/protractor - Could not find Angular on page http://127.0.0.1:8402/ : angular never provided resumeBootstrap

Firefox is the one browser thats relatively convenient to run in CI. Would be nice if that could be fixed even during LTS.

@zeha
Copy link
Author

zeha commented Mar 2, 2021

A temporary workaround for Protractor with FirefoxDriver appears to be:

     capabilities: {
         browserName: 'firefox',
         'moz:firefoxOptions': {
+            prefs: {'privacy.window.name.update.enabled': false}
         }
     },

Mozilla bug report for enabling this feature: https://bugzilla.mozilla.org/show_bug.cgi?id=1685089

Note that it says:
"File follow-up bug for removal of privacy.window.name.update.enabled once it's been flipped for a couple of releases."

@gkalpak
Copy link
Member

gkalpak commented Mar 3, 2021

Thx for reporting this, @zeha.

We need to do some investigation to determine what can/should be done about this (esp. given the current LTS status of the project).

In the meantime, I very briefly looked at the Protractor source code and wanted to capture my findings (for future reference).

Currently, protractor adds NG_DEFER_BOOTSTRAP! to the window name right before setting the window.location to the new URL (source code):

this.executeScriptWithDescription(
    'window.name = "' + DEFER_LABEL + '" + window.name;' +
    'window.location.replace("' + destination + '");',

I experimented with moving updating the window name after the browser has navigated to the new URL (as shown below) and (while it worked fine for most tests) it did cause some test (from the angular/angular.js repo) to fail. I am not sure whether there is a more reliable way to execute some JS code after the new page has been loaded but before any JS (including AngularJS) has been executed.

Modified protractor/lib/browser.ts:

         .then(() => {
-          // Set defer label and navigate
+          // Navigate
           return this.executeScriptWithDescription(
-              'window.name = "' + DEFER_LABEL + '" + window.name;' +
-                  'window.location.replace("' + destination + '");',
+              'window.location.replace("' + destination + '");',
               msg('reset url'));
         })
         .then(() => {
           // We need to make sure the new url has loaded before
           // we try to execute any asynchronous scripts.
           return this.driver.wait(() => {
             return this.executeScriptWithDescription('return window.location.href;', msg('get url'))
                 .then(
                     (url: any) => {
                       return url !== this.resetUrl;
                     },
                     (err: IError) => {
                       if (err.code == 13 || err.name === 'JavascriptError') {
                         // Ignore the error, and continue trying. This is
                         // because IE driver sometimes (~1%) will throw an
                         // unknown error from this execution. See
                         // https://github.com/angular/protractor/issues/841
                         // This shouldn't mask errors because it will fail
                         // with the timeout anyway.
                         return false;
                       } else {
                         throw err;
                       }
                     });
           }, timeout, 'waiting for page to load for ' + timeout + 'ms');
         })
+        .then(() => {
+          // Set defer label
+          return this.executeScriptWithDescription(
+              'window.name = "' + DEFER_LABEL + '" + window.name;',
+              msg('set defer label'));
+        })

@gkalpak
Copy link
Member

gkalpak commented Mar 23, 2021

We discussed this a bit more yesterday and there doesn't seem to be a great way to fix this (i.e. one that is straight-forward and does not involve the risk of breaking other usecases) 🤔

For now, using the work-around mentioned in #17117 (comment) should be sufficient. We might re-visit this if something changes in the future (for example, if the work-around stops being available).

I'll keep the issue open for tracking purposes.

@zeha
Copy link
Author

zeha commented Mar 25, 2021

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1700931 for Firefox (Marionette).

@Splaktar
Copy link
Member

Splaktar commented Apr 3, 2021

I wanted to find a Protractor issue for this, since it's more of an issue there than with AngularJS itself. Related issues

@Splaktar
Copy link
Member

Splaktar commented Apr 3, 2021

From the Firefox bug tracker:

using the pref is not really an option given that it was only added for rollout purposes.

So it looks like that work-around may only work for another couple of months.

@annevk
Copy link

annevk commented Apr 12, 2021

Yeah, I don't know exactly when we'll remove the preference and we at Firefox are happy to collaborate on making that as painless as possible for everyone involved (please chime in at https://bugzilla.mozilla.org/show_bug.cgi?id=1704469), but this window.name dependency was already breaking Safari and will soon also break Chrome so it's high time to migrate to something new.

@mgol
Copy link
Member

mgol commented Apr 12, 2021

@annevk Thanks for the update.

Is https://bugs.chromium.org/p/chromium/issues/detail?id=1090128 a relevant Chromium issue so that we have all the relevant references?

EDIT: I found Chromium's Intent to Ship in a separate issue: https://groups.google.com/a/chromium.org/g/blink-dev/c/86VeIi5sZzc/m/fPOAbuRHCAAJ

@Splaktar
Copy link
Member

Splaktar commented Jun 7, 2021

Chromium is getting closer to shipping this in a beta:
https://groups.google.com/a/chromium.org/g/blink-dev/c/86VeIi5sZzc/m/CVJadvpqAQAJ

Do we have any plans for working around this?

@IgorMinar
Copy link
Contributor

Just a quick not that the current workaround for this issue is to disable the new privacy firefox feature via webdriver configuration:

exports.config = {
  ...
  capabilities: {
    ...
	browserName: 'firefox',
	'moz:firefoxOptions': {
         prefs: {'privacy.window.name.update.enabled': false}
    }
  },

An alternative (and likely worse) option is to keep on using older version of Firefox in these e2e tests.

We currently can't think of a permanent solution to this problem that wouldn't require modification of the test suite and/or the AngularJS application bootstrap code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants