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.

+    let descriptor = Object.getOwnPropertyDescriptor(object, name);
+    delete descriptor["get"];
+    delete descriptor["set"];
+    descriptor.value = function(...args)
+    {
+      let callStack = new Error().stack;
 
-    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) =>
+      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();
+
+      return new Promise((resolve, reject) =>
+      {
+        func.call(object, ...args, result =>
         {
-          let callStack = new Error().stack;
-          if (typeof args[args.length - 1] == "function")
-            return func.apply(object, 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);
 
-          // 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) =>
+            reject(error);
+          }
+          else
           {
-            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;
-                }
-
-                reject(error);
-              }
-              else
-              {
-                resolve(result);
-              }
-            });
-          });
-        }});
-    }
+            resolve(result);
+          }
+        });
+      });
+    };
+    Object.defineProperty(object, name, descriptor);
   }
 
   function shouldWrapAPIs()
   {
     try
     {
       return !(browser.storage.local.get([]) instanceof Promise);
     }
     catch (error)
     {
     }
 
     return true;
   }
 
   if (shouldWrapAPIs())
   {
-    // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser" object,
-    // but provides the extension API through the "chrome" namespace
+    // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser"
+    // object, but provides the extension API through the "chrome" namespace
     // (non-standard).
     if (typeof browser == "undefined")
       window.browser = chrome;
 
     for (let api of asyncAPIs)
       wrapAPI(api);
   }
 
   // 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];
   }
 }