Closed Bug 1245153 Opened 5 years ago Closed 5 years ago

Make remaining utils functionality modules

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

(Keywords: pi-marionette-server)

Attachments

(18 files)

58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
jmaher
: review+
Details
58 bytes, text/x-review-board-request
pbro
: review+
Details
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.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Blocks: 1245572
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/
Attachment #8715426 - Flags: review?(dburns)
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/
Attachment #8715427 - Flags: review?(dburns)
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/
Attachment #8715428 - Flags: review?(dburns)
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/
Attachment #8715429 - Flags: review?(dburns)
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/
Attachment #8715430 - Flags: review?(dburns)
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/
Attachment #8715432 - Flags: review?(dburns)
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/
Attachment #8715433 - Flags: review?(dburns)
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/
Attachment #8715426 - Flags: review?(dburns) → review+
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
Attachment #8715427 - Flags: review?(dburns) → review+
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
Attachment #8715428 - Flags: review?(dburns) → review+
Attachment #8715429 - Flags: review?(dburns) → review+
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
Attachment #8715430 - Flags: review?(dburns) → review+
Attachment #8715431 - Flags: review?(dburns) → review+
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
Attachment #8715432 - Flags: review?(dburns) → review+
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
Attachment #8715433 - Flags: review?(dburns) → review+
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
Attachment #8715434 - Flags: review?(dburns) → review+
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?
Attachment #8715435 - Flags: review?(dburns)
Comment on attachment 8715436 [details]
MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester

https://reviewboard.mozilla.org/r/33441/#review30255
Attachment #8715436 - Flags: review?(dburns) → review+
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?
Attachment #8715436 - Flags: review+
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?
Attachment #8715437 - Flags: review?(dburns)
Comment on attachment 8715438 [details]
MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester

https://reviewboard.mozilla.org/r/33445/#review30261
Attachment #8715438 - Flags: review?(dburns) → review+
Comment on attachment 8715439 [details]
MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester

https://reviewboard.mozilla.org/r/33447/#review30263
Attachment #8715439 - Flags: review?(dburns) → review+
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
Attachment #8715440 - Flags: review?(dburns) → review+
Attachment #8715441 - Flags: review?(dburns)
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
Attachment #8715441 - Flags: review+
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/
Attachment #8715435 - Flags: review?(dburns)
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/
Attachment #8715436 - Flags: review?(dburns)
Attachment #8715437 - Flags: review?(dburns)
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
Attachment #8715435 - Flags: review?(dburns) → review+
Comment on attachment 8715437 [details]
MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester

https://reviewboard.mozilla.org/r/33443/#review30325
Attachment #8715437 - Flags: review?(dburns) → review+
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
Attachment #8715436 - Flags: review?(dburns) → review+
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/
Attachment #8715426 - Attachment description: MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r?automatedtester → MozReview Request: Bug 1245153 - error.isError must recognise built-in Error prototypes; r=automatedtester
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/
Attachment #8715427 - Attachment description: MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r?automatedtester → MozReview Request: Bug 1245153 - Add error.wrap to wrap Error prototypes; r=automatedtester
Attachment #8715428 - Attachment description: MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r?automatedtester → MozReview Request: Bug 1245153 - Wrap errors before they are passed through the IPC channel; r=automatedtester
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/
Attachment #8715429 - Attachment description: MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r?automatedtester → MozReview Request: Bug 1245153 - Convert EventUtils.js to a module; r=automatedtester
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/
Attachment #8715430 - Attachment description: MozReview Request: Bug 1245153 - Convert atoms.js to a module; r?automatedtester → MozReview Request: Bug 1245153 - Convert atoms.js to a module; r=automatedtester
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/
Attachment #8715431 - Attachment description: MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r?automatedtester → MozReview Request: Bug 1245153 - Convert frame-manager.js to a module; r=automatedtester
Attachment #8715432 - Attachment description: MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r?automatedtester → MozReview Request: Bug 1245153 - Convert interactions.js and elements.js to modules; r=automatedtester
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/
Attachment #8715433 - Attachment description: MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r?automatedtester → MozReview Request: Bug 1245153 - Employ new modules throughout Marionette; r=automatedtester
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/
Attachment #8715434 - Attachment description: MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r?automatedtester → MozReview Request: Bug 1245153 - Add event.js module and remove EventUtils.js and sendkeys.js from manifest; r=automatedtester
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/
Attachment #8715435 - Attachment description: MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r?automatedtester → MozReview Request: Bug 1245153 - Remove testing/marionette/atoms; r=automatedtester
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/
Attachment #8715436 - Attachment description: MozReview Request: Bug 1245153 - Rename elements.js to element.js; r?automatedtester → MozReview Request: Bug 1245153 - Rename elements.js to element.js; r=automatedtester
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/
Attachment #8715437 - Attachment description: MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r?automatedtester → MozReview Request: Bug 1245153 - Rename interactions.js to interaction.js; r=automatedtester
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/
Attachment #8715438 - Attachment description: MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r?automatedtester → MozReview Request: Bug 1245153 - Remove testing/marionette/ChromeUtils.js; r=automatedtester
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/
Attachment #8715439 - Attachment description: MozReview Request: Bug 1245153 - Rename actions.js to action.js; r?automatedtester → MozReview Request: Bug 1245153 - Rename actions.js to action.js; r=automatedtester
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/
Attachment #8715440 - Attachment description: MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r?automatedtester → MozReview Request: Bug 1245153 - Rename frame-manager.js to frame.js; r=automatedtester
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/
Attachment #8715441 - Attachment description: MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r?automatedtester → MozReview Request: Bug 1245153 - Lint testing/marionette/frame.js; r=automatedtester
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
Flags: needinfo?(ato)
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/
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/
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/
Attachment #8716299 - Attachment description: MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset → MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r?pbrosset
Attachment #8716299 - Flags: review?(pbrosset)
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/
Attachment #8716299 - Flags: review?(pbrosset)
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/
Attachment #8716299 - Flags: review?(pbrosset)
Attachment #8716921 - Flags: review?(jmaher)
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.
Attachment #8716299 - Flags: review?(pbrosset) → review+
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/
Attachment #8716299 - Attachment description: MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r?pbrosset → MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r?jmaher
Attachment #8716299 - Flags: review?(jmaher)
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/
Attachment #8716921 - Attachment description: MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r?jmaher → MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r?pbrosset
Attachment #8716921 - Flags: review?(jmaher) → review?(pbrosset)
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.
Attachment #8716921 - Flags: review?(pbrosset) → review+
Attachment #8716299 - Flags: review+
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.
Attachment #8716299 - Flags: review?(jmaher) → review+
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/
Attachment #8716299 - Flags: review?(pbrosset)
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/
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/
Attachment #8716299 - Attachment description: MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r?jmaher → MozReview Request: Bug 1245153 - Make EventUtils.js use aWindow argument for sub-calls consistently; r=jmaher
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/
Attachment #8716921 - Attachment description: MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r?pbrosset → MozReview Request: Bug 1245153 - Use EventUtils.js from mochikit; r=pbrosset
(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?
Attachment #8716299 - Flags: review?(pbrosset)
(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/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/9-10/
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/10-11/
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/8-9/
Attachment #8716299 - Flags: review?(pbrosset)
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/4-5/
Attachment #8716299 - Flags: review?(pbrosset)
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/9-10/
Attachment #8716299 - Flags: review?(pbrosset)
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/5-6/
Attachment #8716299 - Flags: review?(pbrosset)
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/10-11/
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/11-12/
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/10-11/
Attachment #8716299 - Flags: review?(pbrosset)
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/6-7/
Attachment #8716299 - Flags: review?(pbrosset)
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/11-12/
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/12-13/
Attachment #8716299 - Flags: review?(pbrosset)
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/11-12/
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/7-8/
Attachment #8716299 - Flags: review?(pbrosset)
Attachment #8716299 - Flags: review?(pbrosset)
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/12-13/
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/8-9/
Attachment #8716299 - Flags: review?(pbrosset)
Attachment #8716299 - Flags: review?(pbrosset)
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/13-14/
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/9-10/
Attachment #8716299 - Flags: review?(pbrosset)
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/14-15/
Attachment #8716299 - Flags: review?(pbrosset)
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/10-11/
Attachment #8716299 - Flags: review?(pbrosset)
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/12-13/
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/13-14/
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/15-16/
Attachment #8716299 - Flags: review?(pbrosset)
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/11-12/
Attachment #8716299 - Flags: review?(pbrosset)
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/13-14/
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/14-15/
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/16-17/
Attachment #8716299 - Flags: review?(pbrosset)
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/12-13/
Attachment #8716299 - Flags: review?(pbrosset)
You need to log in before you can comment on or make changes to this bug.