Issue 5954 - Read-only properties cannot be assigned in strict mode in Edge

polyfill.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 51 matching lines @@
     {
       object = object[node];
 
       if (!object)
         return;
     }
 
     let func = object[name];
     if (!func)
       return;
-
-    object[name] = function(...args)
+    let oldDescriptor = Object.getOwnPropertyDescriptor(object, name);

[kzar, 2017/11/06 08:52:01]

How about this (untested):

let descriptor = Object.getOwnPropertyDescriptor(object, name);
descriptor.value = function(...args)
{
  ...

[Oleksandr, 2017/11/06 09:05:00]

This doesn't work, because some descriptors can be accessor descriptors. That
means they will have set() and/or get() methods, but no value(). If we also set
value() then a call to defineProperty() will fail, because descriptors cannot be
of both accessor and data types
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje....

[kzar, 2017/11/06 10:12:38]

Which APIs are returned using a getter? I find it surprising that any would be.

[Oleksandr, 2017/11/06 10:14:18]

For example `setUnintstallUrl`, as it says on the line below ;)

[kzar, 2017/11/06 10:37:37]

Ah right, well you could just delete the getter and setters.

let descriptor = Object.getOwnPropertyDescriptor(object, name);
delete descriptor["get"];
delete descriptor["set"];
descriptor.value = function(...args)
{
  ...

This also makes me wonder why did they use a getter? Could the function change
at some point in the future, in which case breaking our wrapper?

[Oleksandr, 2017/11/08 13:58:09]

I guess we'll just have to see. I don't know why they used a getter.

[kzar, 2017/11/10 14:18:20]

What do you think about that Manish?

+    // Some descriptors like setUninstallURL are in fact accessor descriptors.
+    // We convert them to data descriptors.
+    let descriptor = {
+      enumerable: oldDescriptor.enumerable,
+      configurable: oldDescriptor.configurable,
+      writable: oldDescriptor.writable
+    };
+    descriptor.value = (...args) =>

[Manish Jethani, 2017/11/05 10:13:52]

Functions and arrow functions have different semantics. Since this is an API
wrapper here, I think maybe it's better to make it a normal function.

[kzar, 2017/11/06 08:52:01]

Agreed

[Oleksandr, 2017/11/06 09:05:00]

Done.

     {
       let callStack = new Error().stack;
 
       if (typeof args[args.length - 1] == "function")
         return func.apply(object, args);
 
       // If the last argument is undefined, we drop it from the list assuming
       // it stands for the optional callback. We must do this, because we have
       // to replace it with our own callback. If we simply append our own
       // callback to the list, it won't match the signature of the function and
@@ skipping 20 matching lines @@
 
             reject(error);
           }
           else
           {
             resolve(result);
           }
         });
       });
     };
+    Object.defineProperty(object, name, descriptor);
   }
 
   function shouldWrapAPIs()
   {
     try
     {
       return !(browser.storage.local.get([]) instanceof Promise);
     }
     catch (error)
     {
@@ skipping 16 matching lines @@
 
   // 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 (let object of [HTMLCollection, NodeList, StyleSheetList, CSSRuleList])
   {
     if (!(Symbol.iterator in object.prototype))
       object.prototype[Symbol.iterator] = Array.prototype[Symbol.iterator];
   }
 }