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

polyfill.js

Index: polyfill.js
===================================================================
--- a/polyfill.js
+++ b/polyfill.js
@@ -69,48 +69,55 @@
     let func = object[name];
     if (!func)
       return;
-
-    object[name] = function(...args)
-    {
-      let callStack = new Error().stack;
-
-      if (typeof args[args.length - 1] == "function")
-        return func.apply(object, args);
+    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.

 
-      // 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 =>
+    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.

+    {
+      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 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);
+          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 will cause an exception.
+          if (typeof args[args.length - 1] == "undefined")
+            args.pop();
 
-              // Add a more helpful stack trace.
-              error.stack = callStack;
-            }
+          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);
 
-            reject(error);
-          }
-          else
-          {
-            resolve(result);
-          }
-        });
-      });
-    };
+                  // Add a more helpful stack trace.
+                  error.stack = callStack;
+                }
+
+                reject(error);
+              }
+              else
+              {
+                resolve(result);
+              }
+            });
+          });
+        }});
+    }
   }
 
   function shouldWrapAPIs()