Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#6468 closed defect (invalid)

Ensure ABP UI Edge 15 compatibility

Reported by: agiammarchi Assignee:
Priority: Unknown Milestone:
Module: Unknown Keywords:
Cc: greiner, saroyanm Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Even Microsoft Virtual Machines ship with Edge 16 already installed by default, and it's also difficult to setup remote VMs (i.e. webmate) to connect to local folders for debugging purpose.

We need to be sure our JS, as well as our CSS and layout in general, works as meant/expected on Edge 15 since this is still part of our target browsers.

Environment

Windows 10 - Edge 15

Observed behaviour

There are common JavaScript patterns we use, such for/of loops, that throws error in Edge 15 because of the missing Symbol.iterator in the NodeList.prototype.

This is just one error we don't easily spot via VMs and Edge 16, the current default base for Edge, but there might be other JS or CSS issues with the version 15.

Expected behaviour

If we say we target Edge 15, it should just work with all of our code, without throwing errors here or there.

Attachments (1)

Screen Shot 2018-03-12 at 14.32.36.png (246.6 KB) - added by agiammarchi 21 months ago.
a for/of failing on node list selection

Download all attachments as: .zip

Change History (6)

Changed 21 months ago by agiammarchi

a for/of failing on node list selection

comment:1 Changed 21 months ago by agiammarchi

After various tests / checks I've concluded that Edge 15 and Chrome < 51 are actually covered by our code base.

While I'd suggest to better polyfill the absence of Symbol.iterator through this loop instead:

  // Workaround since HTMLCollection, NodeList, StyleSheetList, and CSSRuleList
  // didn't have iterator support before Chrome 51.
  // https://bugs.chromium.org/p/chromium/issues/detail?id=401699
  for (const Class of [HTMLCollection, NodeList, StyleSheetList, CSSRuleList])
  {
    if (!(Symbol.iterator in Class.prototype))
      Object.defineProperty(Class.prototype, Symbol.iterator, {
        configurable: true,
        writable: true,
        value: Array.prototype[Symbol.iterator]
      });
  }

I don't think this ticket should be further investigated.

Summary

The polyfill.js we have in UI uses (copy & paste) same polyfill that adblockpluschrome repository bundles, and ship, in production.

Accordingly, all those collections that would otherwise fail in Edge 15 and Chrome < 51 are patched.

On top of that, all recently changed CSS seems to work just as fine in both Edge 15 and Chrome 49.

comment:2 Changed 21 months ago by agiammarchi

  • Resolution set to fixed
  • Status changed from new to closed

comment:3 Changed 21 months ago by agiammarchi

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:4 Changed 21 months ago by agiammarchi

  • Resolution set to invalid
  • Status changed from reopened to closed

comment:5 Changed 21 months ago by agiammarchi

For clarity sake, even if I've investigated everything was fine in Edge, the initial ticket was technically invalid.

We still need to be sure we all grant Edge 15 works when we implement new features, but that's another story.

Note: See TracTickets for help on using tickets.