The following files are still loaded as subscripts: loader.loadSubScript("chrome://marionette/content/EventUtils.js", utils); loader.loadSubScript("chrome://marionette/content/ChromeUtils.js", utils); loader.loadSubScript("chrome://marionette/content/atoms.js", utils); loader.loadSubScript("chrome://marionette/content/sendkeys.js", utils); loader.loadSubScript("chrome://marionette/content/frame-manager.js"); We should turn them into JS modules.

Due to a previous programming error, error.isError only recognised the base Error prototype. It must also test for the other built-in prototypes, such as TypeError et al. Review commit: https://reviewboard.mozilla.org/r/33421/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33421/

Generally, Error prototypes that are not based on WebDriverError must be wrapped so that they can be serialised across the AsyncMessageChannel. Review commit: https://reviewboard.mozilla.org/r/33423/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33423/

error.wrap acts as a no-op if it is passed a prototype which is already of the WebDriverError prototypal chain. Review commit: https://reviewboard.mozilla.org/r/33425/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33425/

testing/marionette/sendkeys.js has been merged into the new testing/marionette/event.js module, together with testing/marionette/EventUtils.js. There is a lot of functionality still left in this module that we can probably remove, as it is not in use by Marionette. Review commit: https://reviewboard.mozilla.org/r/33427/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33427/

Through some very clever hacking of the arguments to each of the atoms, we are able to contain this in a JS module: Atoms normally extract their arguments directly from the function scoped `arguments' variable, but by explicitly naming `window' as the last argument in the functions' prototype we are able to set the `window' variable used inside. This is obviously a big hack, but it encapsulates the atoms and we are moving away from atoms in the long term. Review commit: https://reviewboard.mozilla.org/r/33429/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33429/

Review commit: https://reviewboard.mozilla.org/r/33431/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33431/

To simplify the dependency chain and reduce the number of duplicate functions in Marionette, a number of functions have been removed from interactions.js and added to elements.js. This makes them more easily re-usable and works around a circular dependency issue. Review commit: https://reviewboard.mozilla.org/r/33433/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33433/

This change removes almost all the remaining uses of loadSubScript and global scope pollution. The only remaining use is for common.js, which is resolved by a later bug for evaluating scripts. Review commit: https://reviewboard.mozilla.org/r/33435/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33435/

Review commit: https://reviewboard.mozilla.org/r/33437/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33437/

Review commit: https://reviewboard.mozilla.org/r/33439/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33439/

Review commit: https://reviewboard.mozilla.org/r/33441/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33441/

Review commit: https://reviewboard.mozilla.org/r/33443/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33443/

This does not appear to be in use. Review commit: https://reviewboard.mozilla.org/r/33445/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33445/

Review commit: https://reviewboard.mozilla.org/r/33447/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33447/

Review commit: https://reviewboard.mozilla.org/r/33449/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33449/

There should be no functional changes here. Review commit: https://reviewboard.mozilla.org/r/33451/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33451/

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/1-2/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/1-2/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/1-2/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/1-2/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/1-2/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/1-2/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/1-2/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/1-2/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/1-2/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/1-2/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/1-2/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/1-2/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/1-2/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/1-2/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/1-2/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/1-2/

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester https://reviewboard.mozilla.org/r/33421/#review30225

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester https://reviewboard.mozilla.org/r/33423/#review30227

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester https://reviewboard.mozilla.org/r/33425/#review30229

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester https://reviewboard.mozilla.org/r/33427/#review30231 ::: testing/marionette/event.js:951 (Diff revision 2) > + debugger; nit: you probably dont want this here.

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester https://reviewboard.mozilla.org/r/33429/#review30235

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester https://reviewboard.mozilla.org/r/33431/#review30237

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester https://reviewboard.mozilla.org/r/33433/#review30243 ::: testing/marionette/elements.js:634 (Diff revision 2) > +} nit: missing `;` ::: testing/marionette/elements.js:659 (Diff revision 2) > + c.x + win.pageXOffset <= vp.right && nit: can you move these to be in line with `vp.left` above

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester https://reviewboard.mozilla.org/r/33435/#review30249

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester https://reviewboard.mozilla.org/r/33437/#review30251

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester https://reviewboard.mozilla.org/r/33439/#review30253 ::: testing/marionette/listener.js:17 (Diff revision 2) > -Cu.import("chrome://marionette/content/atoms.js"); > +Cu.import("chrome://marionette/content/atom.js"); atom implies that we only have 1 atom in there. Why change the name like this?

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester https://reviewboard.mozilla.org/r/33441/#review30255

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester https://reviewboard.mozilla.org/r/33441/#review30257 ::: testing/marionette/driver.js:29 (Diff revision 2) > -Cu.import("chrome://marionette/content/elements.js"); > +Cu.import("chrome://marionette/content/element.js"); The object inside is Elements, why the change?

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester https://reviewboard.mozilla.org/r/33443/#review30259 ::: testing/marionette/driver.js:28 (Diff revision 2) > -Cu.import("chrome://marionette/content/interactions.js"); > +Cu.import("chrome://marionette/content/interaction.js"); The object inside is Interactions, why the change?

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester https://reviewboard.mozilla.org/r/33445/#review30261

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester https://reviewboard.mozilla.org/r/33447/#review30263

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester https://reviewboard.mozilla.org/r/33449/#review30265

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester https://reviewboard.mozilla.org/r/33451/#review30267

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester https://reviewboard.mozilla.org/r/33451/#review30269

https://reviewboard.mozilla.org/r/33441/#review30257 > The object inside is Elements, why the change? There’s no object in this module called `Elements`. The long term plan, however, is to move the various exported symbols onto the `element` object so that we give some impression of module encapsulation: ``` let elementManager = element.Storage(…) element.LocationStrategy.ClassName element.checkVisible(…) element.isXULElement(…) ``` … and so on.

https://reviewboard.mozilla.org/r/33439/#review30253 > atom implies that we only have 1 atom in there. Why change the name like this? It doesn’t really imply this. Just as `element.isXULElement` doesn’t imply there’s only one symbol exposed on `element`, `atom.isElementDisplayed` just means “from the atom module, run this function”. The second point is that all the modules in _testing/marionette_ now follow the same stylistic and linguistic pattern, where singular form is used to describe and bundle together functions that are related to one area. For example `error` exposes multiple different error prototypes, but the module itself exposes functionality as `error.isError` and `error.report`. It’s quite a normal pattern to use singular style to name modules.

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/2-3/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/2-3/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/2-3/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/2-3/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/2-3/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/2-3/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/2-3/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/2-3/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/2-3/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/2-3/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/2-3/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/2-3/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/2-3/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/2-3/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/2-3/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/2-3/

Manual try run because Autoland-to-try is down: https://treeherder.mozilla.org/#/jobs?repo=try&revision=13f5546bc98a

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester https://reviewboard.mozilla.org/r/33439/#review30313

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester https://reviewboard.mozilla.org/r/33443/#review30325

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/3-4/

Addressed a couple of issues and should get test_switch_remote_frame.py passing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f76e0e93605

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester https://reviewboard.mozilla.org/r/33441/#review30369

https://hg.mozilla.org/integration/mozilla-inbound/rev/aec0a0166685 https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5e37f8e446 https://hg.mozilla.org/integration/mozilla-inbound/rev/d75ad1397656 https://hg.mozilla.org/integration/mozilla-inbound/rev/4b2a5ee7199e https://hg.mozilla.org/integration/mozilla-inbound/rev/ee7ee24cc65e https://hg.mozilla.org/integration/mozilla-inbound/rev/bd41e4ab829d https://hg.mozilla.org/integration/mozilla-inbound/rev/5da7628c3767 https://hg.mozilla.org/integration/mozilla-inbound/rev/6bb93562a576 https://hg.mozilla.org/integration/mozilla-inbound/rev/55f64197f6b5 https://hg.mozilla.org/integration/mozilla-inbound/rev/3edb67388ad6 https://hg.mozilla.org/integration/mozilla-inbound/rev/80ae953819c8 https://hg.mozilla.org/integration/mozilla-inbound/rev/c1e0abbfa66e https://hg.mozilla.org/integration/mozilla-inbound/rev/22321e6b65e9 https://hg.mozilla.org/integration/mozilla-inbound/rev/1be5f60393a0 https://hg.mozilla.org/integration/mozilla-inbound/rev/4c2b1902d7cd https://hg.mozilla.org/integration/mozilla-inbound/rev/30c8ec933737

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=21136158&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9f7157c44c01

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/3-4/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/3-4/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/3-4/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/3-4/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/3-4/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/3-4/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/3-4/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/3-4/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/3-4/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/3-4/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/3-4/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/3-4/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/3-4/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/3-4/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/3-4/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/4-5/

testing/marionette/EventUtils.js has been converted to a JS module in testing/marionette/event.js and its API has changed. It was originally a copy of testing/mochitest/tests/SimpleTest/EventUtils.js, and it should be fine to use the original instead. Review commit: https://reviewboard.mozilla.org/r/33781/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33781/

The devtools tests were relying on testing/marionette/EventUtils.js. The latest addition to this patch series changes them to rely on testing/mochitest/tests/SimpleTest/EventUtils.js instead. Try run to confirm that change is fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc8f90fdbba2&group_state=expanded

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/4-5/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/4-5/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/4-5/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/4-5/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/4-5/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/4-5/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/4-5/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/4-5/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/4-5/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/4-5/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/4-5/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/4-5/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/4-5/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/4-5/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/4-5/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/5-6/

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/1-2/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=660dcd048efb

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=761c7c50e81d

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/2-3/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6633f71ed64

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/5-6/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/5-6/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/5-6/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/5-6/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/5-6/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/5-6/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/5-6/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/5-6/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/5-6/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/5-6/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/5-6/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/5-6/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/5-6/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/5-6/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/5-6/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/6-7/

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/3-4/

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/6-7/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/6-7/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/6-7/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/6-7/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/6-7/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/6-7/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/6-7/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/6-7/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/6-7/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/6-7/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/6-7/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/6-7/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/6-7/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/6-7/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/6-7/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/7-8/

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/4-5/

EventUtils.js previously allowed you to override the Window object reference through passing it as an optional argument to its functions. This change fixes certain uses of implicit globals that reside on Window. Review commit: https://reviewboard.mozilla.org/r/33991/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33991/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48a0b894185b

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher https://reviewboard.mozilla.org/r/33781/#review30603 This seems like a lot of duplicated lines of code to make a simple helper script work. Other tests in the tree that use chrome://mochikit/content/tests/SimpleTest/EventUtils.js seem to do the same, and if try is happy, I'm happy. We should remove some of the instances of devtools tests that need this and make them use BrowserTestUtils instead. I'll file another bug for us to do that.

https://reviewboard.mozilla.org/r/33781/#review30605 ::: devtools/client/shared/frame-script-utils.js:24 (Diff revision 5) > +subScriptLoader.loadSubScript("chrome://mochikit/content/tests/SimpleTest/EventUtils.js", EventUtils); I just realized that in this frame-script, the only message listener that uses EventUtils is Test:SynthesizeKey, and it turns out that in devtools, no existing test uses this message anymore. So instead, you could just delete the EventUtils import and delete the Test:SynthesizeKey message listener.

https://reviewboard.mozilla.org/r/33781/#review30603 I had two options in this case: Either copy the old EventUtils.js from Marionette into the devtools tree, or try to fix the new EventUtils.js in mochitest to be compatible with the way the devtools tests that had relied on Marionette’s version uses it. I chose the latter to avoid any more duplication than strictly necessary. I obviously agree entirely with replacing EventUtils.js altogether with something more self-contained and reusable.

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/5-6/

Comment on attachment 8716921 [details] MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33991/diff/1-2/

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de5761a35fa8

Comment on attachment 8716921 [details] MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset https://reviewboard.mozilla.org/r/33991/#review30619 Thanks for the cleanup.

This is a digression. (In reply to Andreas Tolfsen (:ato) from comment #155) > I obviously agree entirely with replacing EventUtils.js altogether with > something more self-contained and reusable. In fact, it’s quite possible that ‘this something’ should live in Marionette and be a normal JS module that is _wrapped_ by mochitest as a subscript, so that the back-end could be re-used by everyone; regardless of whether you want the functionality in the same namespace or as a module.

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher https://reviewboard.mozilla.org/r/33781/#review30763 thanks for writing this1

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/7-8/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/7-8/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/7-8/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/7-8/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/7-8/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/7-8/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/7-8/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/7-8/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/7-8/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/7-8/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/7-8/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/7-8/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/7-8/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/7-8/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/7-8/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/8-9/

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/6-7/

Comment on attachment 8716921 [details] MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33991/diff/2-3/

https://hg.mozilla.org/integration/mozilla-inbound/rev/36526a2e8b00 https://hg.mozilla.org/integration/mozilla-inbound/rev/dc6460e0f336 https://hg.mozilla.org/integration/mozilla-inbound/rev/eff85dc0eaa9 https://hg.mozilla.org/integration/mozilla-inbound/rev/048d70070751 https://hg.mozilla.org/integration/mozilla-inbound/rev/e41a5b41859a https://hg.mozilla.org/integration/mozilla-inbound/rev/6ebd9fde50c0 https://hg.mozilla.org/integration/mozilla-inbound/rev/68a6dda373d2 https://hg.mozilla.org/integration/mozilla-inbound/rev/ad5bf32d8fef https://hg.mozilla.org/integration/mozilla-inbound/rev/b3be2d2f3ac1 https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe42d498a2a https://hg.mozilla.org/integration/mozilla-inbound/rev/63a56310a1b5 https://hg.mozilla.org/integration/mozilla-inbound/rev/6da8099573f3 https://hg.mozilla.org/integration/mozilla-inbound/rev/3b9e9a027fa7 https://hg.mozilla.org/integration/mozilla-inbound/rev/9f3f1c358e47 https://hg.mozilla.org/integration/mozilla-inbound/rev/f79252e92acc https://hg.mozilla.org/integration/mozilla-inbound/rev/c29a348930a4 https://hg.mozilla.org/integration/mozilla-inbound/rev/98b6d0c053c0 https://hg.mozilla.org/integration/mozilla-inbound/rev/18d54b8d4ae8

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=21529039&repo=mozilla-inbound

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/8-9/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/8-9/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/8-9/

Comment on attachment 8715429 [details] MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33427/diff/8-9/

Comment on attachment 8715430 [details] MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33429/diff/8-9/

Comment on attachment 8715431 [details] MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33431/diff/8-9/

Comment on attachment 8715432 [details] MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33433/diff/8-9/

Comment on attachment 8715433 [details] MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33435/diff/8-9/

Comment on attachment 8715434 [details] MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33437/diff/8-9/

Comment on attachment 8715435 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33439/diff/8-9/

Comment on attachment 8715436 [details] MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33441/diff/8-9/

Comment on attachment 8715437 [details] MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33443/diff/8-9/

Comment on attachment 8715438 [details] MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33445/diff/8-9/

Comment on attachment 8715439 [details] MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33447/diff/8-9/

Comment on attachment 8715440 [details] MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33449/diff/8-9/

Comment on attachment 8715441 [details] MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33451/diff/9-10/

Comment on attachment 8716299 [details] MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33781/diff/7-8/

Comment on attachment 8716921 [details] MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33991/diff/3-4/

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/2baea2550106

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0706694d6d67

(In reply to Andreas Tolfsen ‹:ato› from comment #178) > Comment on attachment 8716299 [details] > MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for > sub-calls consistently; r=jmaher > > Review request updated; see interdiff: > https://reviewboard.mozilla.org/r/33781/diff/6-7/ Did you intend to ask me for review on this patch? It's been reviewed already, and you've landed the changes since then, so I guess this was a mistake?

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #202) > Did you intend to ask me for review on this patch? It's been reviewed > already, and you've landed the changes since then, so I guess this was a > mistake? Oh sorry, no I did not. This is a bug in MozReview that I’ve reported. Again I’m terribly sorry about the noise.

Comment on attachment 8715426 [details] MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33421/diff/9-10/

Comment on attachment 8715427 [details] MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33423/diff/9-10/

Comment on attachment 8715428 [details] MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33425/diff/9-10/