Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

browser.devtools.inspectedWindow.eval() resolves with incorrect promise signature #168

Closed
keithclark opened this issue Jan 10, 2019 · 7 comments · Fixed by #175
Closed
Assignees

Comments

@keithclark
Copy link

When calling browser.devtools.inspectedWindow.eval() in Chrome (I'm using 71), the resulting promise appears to resolve with an incorrect signature. According to MDN, the resulting promise should be an array containing two elements:

A Promise that will be fulfilled with an array containing two elements.

If no error occurred, element 0 will contain the result of evaluating the expression, and element 1 will be undefined.

If an error occurred, element 0 will be undefined, and element 1 will contain an object giving details about the error. Two different sorts of errors are distinguished

However, running the following script

let statement = `Array.from(document.querySelectorAll('*')).map(e=>e.tagName)`;
browser.devtools.inspectedWindow.eval(statement).then(console.log);

Returns

["HTML", "HEAD", "META", ...]

I was expecting

[
  ["HTML", "HEAD", "META", ...],
  undefined
]

If I call the same function with a statement that will throw an error, the promise resolves with the correct error signature:

let statement = `%%%`;
browser.devtools.inspectedWindow.eval(statement).then(console.log);

Returns:

[
  undefined,
  {
    isException: true,
    value: "SyntaxError: Unexpected token %"
  }
]

Firefox behaves as expected.

@Rob--W
Copy link
Member

Rob--W commented Jan 11, 2019

Chrome's devtools API implementation is not implemented using the default extension bindings. The implementation is here, and contrary to Chrome's documentation, there are not always two parameters passed to the callback: https://chromium.googlesource.com/chromium/src/+/10e9b59ba1acc764a058b8d152e3825ff7db50ba/third_party/blink/renderer/devtools/front_end/extensions/ExtensionAPI.js#577

I suppose that we could repurpose singleCallbackArg in our polyfill, so that:

  • true = force single value (=current implementation)
  • unset = array or single value depending on number of arguments (=current implementation)
  • false = force array (=new behavior, to fix this bug).

@keithclark
Copy link
Author

keithclark commented Jan 11, 2019

If I understand the Chromium source correctly, the 2nd parameter isn't passed when the eval is successful. I guess that's not really an issue for the event hander as omitting the 2nd argument would still result in value of undefined when trying to access element [1] of the array.

Repurposing singleCallbackArg as you suggested would work.

Just a suggestion: Would it make the code easier to understand if you used string values instead? Something like:

  • single = force single value (=current implementation)
  • auto = array or single value depending on number of arguments (=current implementation)
  • array = force array (=new behavior, to fix this bug).

@frehner
Copy link

frehner commented Feb 5, 2019

Oh nice, I just barely ran into this as well. Glad to see that it's already been fixed and merged! Awesome work.

Any idea on when the next release is so I can use it?

@frehner
Copy link

frehner commented Feb 6, 2019

workaround until this is released:

  if (typeof InstallTrigger !== "undefined") {
    // see https://stackoverflow.com/a/41820692/2098017
    return result[0];
  } else {
    return result;
  }

@rpl
Copy link
Member

rpl commented Feb 6, 2019

Any idea on when the next release is so I can use it?

@frehner we just released this fix on npm as part of the new webextension-polyfill 0.4.0 version.

@joelstransky
Copy link

I'm confused. How are you calling browser.devtools.inspectedWindow.eval() in chrome when in chrome it's chrome.devtools.inspectedWindow.eval(). Also, how is this relevant if chrome's eval doesn't even return a promise?

@Rob--W
Copy link
Member

Rob--W commented Mar 16, 2020

@joelstransky This project (webextension-polyfill) provides a browser namespace to environments without such an environment, which provides promisified methods for APIs that are known to have a callback. devtools.inspectedWindow.eval is such an API. See this project's README for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants