search

mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
564 stars 64 forks source link

ensure iframe first; set toolboxOpened unconditionally; end after catch #50

Closed mykmelez closed 8 years ago

mykmelez commented 8 years ago

@jryans While resolving conflicts with a merge from upstream, I noticed a few issues with toolbox-init:

  1. The script tests if (iframe) after calling iframe.QueryInterface(), but that call should throw an exception if iframe is falsy, so the conditional will never be false.
  2. After fixing issue #1 by testing iframe before calling iframe.QueryInterface(), if iframe is falsy, then toolboxOpened is never defined, so the toolboxOpened.catch() call below will fail.
  3. The promise chain will reach the toolboxOpened.then() function even if there was an error opening the toolbox, since it gets attached to the chain after toolboxOpened.catch(), which doesn't re-throw the error.

Here are fixes for these issues.

jryans commented 8 years ago

Looks good to me, thanks!