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;
+    let descriptor = Reflect.getOwnPropertyDescriptor(object, name);

[Oleksandr, 2017/10/27 13:12:37]

We cannot use Object.getOwnPropertyDescriptor because it is not defined for some
objects.

[Manish Jethani, 2017/10/27 14:25:16]

We can use Object.getOwnPropertyDescriptor here since we know for sure that the
target won't be undefined.

Nit: I would add a blank line after the return statement above this, but your
choice.

 
-    object[name] = function(...args)
+    if (descriptor && descriptor.configurable)

[Oleksandr, 2017/10/27 13:12:38]

There isn't really an object in our list that doesn't satisfy these
requirements, so maybe this check can be removed.

[Manish Jethani, 2017/10/27 14:25:16]

Yes, we should remove it.

If it's not configurable it'll fail during development and we'll know we need to
figure something out for it.

     {
-      let callStack = new Error().stack;
+      Object.defineProperty(object, name, {

[Manish Jethani, 2017/10/27 14:25:16]

You could just do:

  descriptor.value = (...args) =>
  {
    ...
  };

  Object.defineProperty(object, name, descriptor);

This way we don't have to hard-code the values for enumerable, writable, and
configurable, they will be what they are on that platform.

+        enumerable: true,
+        configurable: true,
+        writable: true,
+        value: (...args) =>
+        {
+          let callStack = new Error().stack;
+          if (typeof args[args.length - 1] == "function")
+            return func.apply(object, args);
 
-      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 will cause an exception.
+          if (typeof args[args.length - 1] == "undefined")
+            args.pop();
 
-      // 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
-      // will cause an exception.
-      if (typeof args[args.length - 1] == "undefined")
-        args.pop();
+          return new Promise((resolve, reject) =>
+          {
+            func.call(object, ...args, result =>
+            {
+              let error = browser.runtime.lastError;
+              if (error && !portClosedBeforeResponseError.test(error.message))
+              {
+                // runtime.lastError is already an Error instance on Edge, while
+                // on Chrome it is a plain object with only a message property.
+                if (!(error instanceof Error))
+                {
+                  error = new Error(error.message);
 
-      return new Promise((resolve, reject) =>
-      {
-        func.call(object, ...args, result =>
-        {
-          let error = browser.runtime.lastError;
-          if (error && !portClosedBeforeResponseError.test(error.message))
-          {
-            // runtime.lastError is already an Error instance on Edge, while on
-            // Chrome it is a plain object with only a message property.
-            if (!(error instanceof Error))
-            {
-              error = new Error(error.message);
+                  // Add a more helpful stack trace.
+                  error.stack = callStack;
+                }
 
-              // Add a more helpful stack trace.
-              error.stack = callStack;
-            }
-
-            reject(error);
-          }
-          else
-          {
-            resolve(result);
-          }
-        });
-      });
-    };
+                reject(error);
+              }
+              else
+              {
+                resolve(result);
+              }
+            });
+          });
+        }});
+    }
   }
 
   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];
   }
 }