Issue 7294 - Pass necessary functions to strip-fetch-query-parameter

lib/content/snippets.js

Index: lib/content/snippets.js
===================================================================
--- a/lib/content/snippets.js
+++ b/lib/content/snippets.js
@@ -856,25 +856,28 @@
  * Strips a query string parameter from <code>fetch()</code> calls.
  *
  * @param {string} name The name of the parameter.
  * @param {?string} [urlPattern] An optional pattern that the URL must match.
  */
 function stripFetchQueryParameter(name, urlPattern = null)
 {
   let fetch_ = window.fetch;
+  if (typeof fetch_ != "function")
+    return;
+
   let urlRegExp = urlPattern ? toRegExp(urlPattern) : null;
   window.fetch = function fetch(...args)
   {
     let [source] = args;
     if (typeof source == "string" &&
         (!urlRegExp || urlRegExp.test(source)))
     {
       let url = new URL(source);
       url.searchParams.delete(name);
       args[0] = url.href;
     }
     return fetch_.apply(this, args);

[a.giammarchi, 2019/02/21 10:26:58]

cannot we just `return fetch_(...args)` after latest discussion?
alternatively patch `apply` via bind right away and `return apply(fetch_, this,
args)`?

example
```
let fetch_ = window.fetch;
// ...
function fetch(...args) {
  // ....
  return apply(fetch_, this, args);
}
let apply = fetch.call.bind(fetch.bind)(fetch.call, fetch.apply);
window.fetch = fetch;
```
that ensure, unless they have modified Function.prototype.apply/bind/call that
no matter how they patched fetch, it's invoked.

It suffers same "maybe they wrapped apply/call/bind" but in case it's executed
before they do that, and if there is only one param to removed, it'll be secured
"forever".

What do you think?

[Manish Jethani, 2019/02/21 10:35:04]

I don't see why we should assume anything about the fetch function. Maybe it is
completely different than the fetch API that we expect. Maybe there's a custom
implementation that requires `this`. How do we know? We should just pass on the
`this` that we get instead of trying to "correct" it; that is the decent thing
to do. It is not the snippet's business to decide whether the fetch function in
the document should get a `this` -- if the caller has passed a `this`, we should
pass it on.

This applies in general to all API wrappers. We need to interfere as little as
possible with the  code.

Anyway, since we decided not to make any more changes in this patch, let's take
this discussion elsewhere.

   };
 }
 
-exports["strip-fetch-query-parameter"] =
-  makeInjector(stripFetchQueryParameter);
+exports["strip-fetch-query-parameter"] = makeInjector(stripFetchQueryParameter,
+                                                      toRegExp, regexEscape);