1448045 - Can we remove the window.controllers shim?

Status: NEW

(Core :: DOM: Core & HTML, task, P3)

Description

[Boris Zbarsky [:bzbarsky]]

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-03-01&keys=__none__!__none__!__none__&max_channel_version=beta%252F59&measure=USE_COUNTER2_DEPRECATED_Window_Cc_ontrollers_PAGE&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2018-01-22&table=0&trim=1&use_submission_date=0 shows .07% of pageloads actually used it.  

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2018-03-01&keys=__none__!__none__!__none__&max_channel_version=beta%252F59&measure=USE_COUNTER2_DEPRECATED_Window_Cc_ontrollers_DOCUMENT&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2018-01-22&table=0&trim=1&use_submission_date=0 says that it was .01% of documents.

These are both on Firefox 59 beta, because we had a full cycle of that, but the 60 beta numbers are similar.

Mike, have you seen any web compat issues reported due to nightly not having this shim?  This is the thing that adds a window.controllers and window.Controllers in beta-and-release only.

Comment 1

[Boris Zbarsky [:bzbarsky]]

But note bug 1010577, where removal did not go _that_ well last time.  But some of the relevant libraries got fixed since then, so maybe we can do better now...  We would need to at least check the sites that caused this shim to be put in place.

Comment 2

[Mike Taylor [:miketaylr]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #0)
> Mike, have you seen any web compat issues reported due to nightly not having
> this shim?  This is the thing that adds a window.controllers and
> window.Controllers in beta-and-release only.

The only instance where it's shown up is https://github.com/webcompat/web-bugs/issues/12639#issuecomment-338226644, but it seems unrelated to the bug as reported (and I just confirmed that app still works in Nightly). AFAICT, the fallout for Nightly users has been minimal.

Since it was coming from ace.js, I see they updated their detection code in the following commit:

https://github.com/ajaxorg/ace/commit/ee9bdb7849651a83f59c4cd237aef39f74b0a2e8#diff-3a58a665ec60f7a8cf535c7590d27580

I'll ask our SV contractors to go over the sites from 1010577 and report back here. Leaving ni? to remember to do that.

Comment 3

[Sergiu Logigan [:sergiu]]

Hello all,

We took a look on https://bugzilla.mozilla.org/show_bug.cgi?id=1010577 and came up with a list of sites that we thought needed testing to see if any of them log a "window.controllers/Controllers is deprecated. Do not use it for UA detection." message in the console.

Here are our findings:

#List of sites#
https://ib.absa.co.za/
https://ib.absa.co.za/absa-online/login.jsp
http://www.absa.co.za/Absacoza/Contact-Us
http://www.kpnvandaag.nl/
http://www.kpnvandaag.nl/static/lib/bcf/4_4_8/engine/backbase.js
https://forschung.beuth-hochschule.de/bis/beuth
http://livehttpheaders.mozdev.org
http://livehttpheaders.mozdev.org/bugs.html
https://www.gmx.com/
http://portableapps.com/node/41388
https://www.edx.org/courses/MITx/6.002x/2012_Fall/courseware/Week_1/Circuit_Analysis_Toolchest/
http://en.wikipedia.org/wiki/Simple_linear_regression
http://bits.wikimedia.org/static-1.20wmf11/extensions/Math/modules/MathJax/jax/output/HTML-CSS/jax.js
http://www.mathjax.org/demos/tex-samples/
https://www.gnu.org/software/automake/manual/html_node/Multiple-Outputs.html
https://www.eff.org/files/https-everywhere-4.0.1.xpi
https://www.1and1.com/
https://sd2.1und1.de/guest?path=Test%20von%2067171311&token=BE869FABF25BEE47&mandant=06&locale=de&viewType=0

#Particularities#
https://ib.absa.co.za/absa-online/login.jsp - displays a "Browser Unsupported" message on Nightly 61.
https://forschung.beuth-hochschule.de/bis/beuth - "Secure Connection Failed" on Nightly 61, "Unable to connect" on Release 59.
https://www.edx.org/courses/MITx/6.002x/2012_Fall/courseware/Week_1/Circuit_Analysis_Toolchest/ - "404" page on both Nightly 61 and Release 59.
http://bits.wikimedia.org/static-1.20wmf11/extensions/Math/modules/MathJax/jax/output/HTML-CSS/jax.js - "We’re having trouble finding that site" page on both Nightly 61 and Release 59.
https://www.eff.org/files/https-everywhere-4.0.1.xpi - Could not open on both Nightly 61 and Release 59.

#window.controllers findings#
Only https://ib.absa.co.za/absa-online/login.jsp logs the "window.controllers/Controllers is deprecated. Do not use it for UA detection." message in the Console, on Firefox Release 59.
On Nightly 61, none of them log this message.

I don't know if we caught every site in question, but if we didn't we'll be happy to comb through Bugzilla again.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> On Nightly 61, none of them log this message.

Right, on Nightly 61 we hid Controllers entirely.

Looks like https://ib.absa.co.za/absa-online/login.jsp does in fact use Controllers and breaks if it's not present.  The use looks like this:

    if ((document.all || (window.fF_ && document.all instanceof window.fF_)) && !navigator.userAgent.match(/opera/i) && !navigator.appVersion.match(/mac/i)) {
      bb.A_ = true;
      bb.cd_ = "ie";
      /* etc */

    } else {
      if ((window['controllers'] || window['Controllers']) && parseInt(navigator.productSub, 10) > 20031001) {
        bb._T_ = true;
        bb.cd_ = 'gecko';
        bb._m_ = parseFloat(navigator.userAgent.substr(navigator.userAgent.indexOf('rv:') + 3, 4))
      } else {
        if (navigator.userAgent.match(/opera/i)) {
          bb._N_ = true;
          bb.cd_ = 'opera';
          bb._m_ = parseFloat(navigator.userAgent.substr(navigator.userAgent.indexOf(/opera[ \/]/i) + 7, 5))
        } else {
          if (navigator.userAgent.match(/applewebkit/i)) {
            bb._E_ = true;
            bb.cd_ = 'webkit';
            bb._m_ = parseFloat(navigator.userAgent.match(/applewebkit\/([0-9\.]+)/i) [1]);
            bb.fG_ = navigator.userAgent.match(/AdobeAIR/) && !!window.runtime
          }
        }
      }
    }

and then later:

    if (!(bb.A_ || bb._T_ || bb._N_ || bb._E_)) {
      return bb.customFallback(1)
    }

which is where the "unsupported" alert comes from.  So the only way to work with this site while dropping Controllers is to have either "webkit" or "opera" in the UA string (or have a non-falsy document.all, which no one does at this point).

Sergiu, thank you for digging through those!

Comment 5

[Bobby Holley (:bholley)]

Do we support per-origin UA overrides on Desktop? If so, this seems like a great use-case.

Comment 6

[Mike Taylor [:miketaylr]]

(In reply to Bobby Holley (:bholley) from comment #5)
> Do we support per-origin UA overrides on Desktop? If so, this seems like a
> great use-case.

I believe we do! 

(Or soon will... Dennis, is this possible today or do we need to wait for Bug 1386807 to land?)

Comment 7

[Dennis Schubert [:denschub]]

> Dennis, is this possible today

We could do that right now, with our Version 1 source in all the browsers, and rolling out an update based on the old code. Do you want me to run some tests on the UA changes we need to make and propose an update to be rolled out?

Comment 8

[Mike Taylor [:miketaylr]]

Yes, please. Can you file a bug for that?

Comment 9

[Dennis Schubert [:denschub]]

Done, bug 1452707. Will build the patch and push the XPI to QA as my first action tomorrow.