879972 - Create new dialog ui class (which also drops the usage of lookup expressions)

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Jonathan French (:jfrench)]

This is a marker bug to create new dialog ui class, to support the eventual removal and replacement of elementslib.Lookup from affected tests which use that method.

An example of which, is lib/tests/testSearch.js
http://hg.mozilla.org/qa/mozmill-tests/file/b8a22c8c9a3b/lib/tests/testSearch.js#l76

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Well, in that new dialog ui class we should not make use of lookup strings but get those elements via the nodeCollector.

Comment 2

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v1.0

I added the BaseDialog class as discussed, and re-factored the cases where we had Lookup expressions. For the modals that are opened via prompt-service we can't use the opening method that I added, but we still can pass in the controller from handling function passed in modalDialog.start

Comment 3

[Mihaela Velimiroviciu (:mihaelav)]

Comment on attachment 8546492 [details] [diff] [review]
patch v1.0

Review of attachment 8546492 [details] [diff] [review]:
-----------------------------------------------------------------

Please update the commit message.

The following tests are failing (test timeout) with the new code from search.js - installFromUrl:
* firefox/tests/functional/testSearch/testAddMozSearchProvider.js
* firefox/tests/functional/testSearch/testSearchSelection.js
* firefox/tests/functional/testSearch/testSearchSuggestions.js

::: firefox/lib/ui/browser.js
@@ +252,5 @@
> + *
> + * @param {string} aURL
> + *         URL of the file which has to be downloaded
> + */
> +BrowserWindow.prototype.openBaseDialog = function BW_openBaseDialog(aCallback) {

This should be placed before the openPageInfoWindow function in order to keep the alphabetical order.

::: lib/tests/testBaseDialog.js
@@ +4,5 @@
> +
> +"use strict";
> +
> +// Include required modules
> +var utils = require("../utils");

I don't see where you use utils library in this test, please remove.

::: lib/ui/base-dialog.js
@@ +26,5 @@
> + * Retrieve list of UI elements based on the given specification
> + *
> + * @param {object} aSpec
> + *        Information of the UI elements which should be retrieved
> + * @parma {string} aSpec.type

nit: @param {string} [aSpec.type]

@@ +32,5 @@
> + * @param {string} [aSpec.subtype]
> + *        Attribute of the elements to filter
> + * @param {string} [aSpec.value]
> + *        Value of the attribute to filter
> + * @param {string} [aSpec.parent=document]

Please order the params

Comment 4

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v2.0

(In reply to Mihaela Velimiroviciu [QA] (:mihaelav) from comment #3)see where you use utils library in this test, please remove.
> > + * Retrieve list of UI elements based on the given specification
> > + *
> > + * @param {object} aSpec
> > + *        Information of the UI elements which should be retrieved
> > + * @parma {string} aSpec.type
> 
> nit: @param {string} [aSpec.type]
We put brackets around the param if it is optional, aSpec.type is not optional is mandatory, it will raise an exception if is not provided
 
> @@ +32,5 @@
> > + * @param {string} [aSpec.subtype]
> > + *        Attribute of the elements to filter
> > + * @param {string} [aSpec.value]
> > + *        Value of the attribute to filter
> > + * @param {string} [aSpec.parent=document]
> 
> Please order the params
I don't think that we order function arguments alphabetically, usually we order them from mandatory to optional so the less probable is for parameter to be offered the latter it will be in order, so you avoid having calls like functionName(undefined, undefined, "mandatoryParameter")

http://mozmill-crowd.blargon7.com/#/remote/report/5bb4668cfa6af53a0ba505835d12932d
http://mozmill-crowd.blargon7.com/#/functional/report/5bb4668cfa6af53a0ba505835d104c36

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8546492 [details] [diff] [review]
patch v1.0

Review of attachment 8546492 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks great, Cosmin! There are some things to clarify and fix, but that should be done kinda quick. Also I wonder if those were already all the existing tests we have used the commondialog.

::: firefox/lib/search.js
@@ -180,5 @@
> -   * @private
> -   */
> -  _handleEngineInstall : function engineManager_handleEngineInstall(aController) {
> -    var confirmTitle = utils.getProperty("chrome://global/locale/search/search.properties",
> -                                         "addEngineConfirmTitle");

As you can see this is a special dialog, and needs its own class.

@@ +298,5 @@
>  
>      this._controller.open(aUrl);
>      this._controller.waitForPageLoad();
>  
> +    var dialog = baseDialog.open(aAddEngineCallback);

BaseDialog is not going to replace any custom dialog. Here we would need a specific implementation. So this needs to be reverted.

Also is this not a modal dialog anymore? I ask because you completely removed the handling code.

::: firefox/lib/ui/browser.js
@@ +253,5 @@
> + * @param {string} aURL
> + *         URL of the file which has to be downloaded
> + */
> +BrowserWindow.prototype.openBaseDialog = function BW_openBaseDialog(aCallback) {
> +  return baseDialog.open(aCallback);

I do not see what the idea of this method is. If there is a method which opens a dialog, we will need custom code.

::: firefox/tests/functional/restartTests/testPreferences_masterPassword/test2.js
@@ +108,3 @@
>  
> +  var passwordBox = dialog.getElement({type: "password_textbox"});
> +  passwordBox.waitForElement();

Do we still have to wait here? Maybe we need another event specifically for the commondialog to wait for its completed load.

::: firefox/tests/remote/testSecurity/testSubmitUnencryptedInfoWarning.js
@@ +88,4 @@
>  
>    // Wait for the content to load
> +  var infoBody = dialog.getElement({type: "info_body"});
> +  infoBody.waitForElement();

Do we still have to wait?

::: lib/tests/testBaseDialog.js
@@ +24,5 @@
> +  windows.closeAllWindows(aModule.browserWindow);
> +}
> +
> +function testBaseDialog() {
> +  var dialog = browserWindow.openBaseDialog(() => {

As already mentioned in the browser class, this is not a function we want to have. So use a very basic opening via base_dialog.open().

@@ +31,5 @@
> +  });
> +
> +  TEST_ELEMENTS.forEach((aElement) => {
> +    var element = dialog.getElement({type: aElement});
> +    assert.ok(element && element.exists(),

You want an expect here so we can test all of the elements.

::: lib/ui/base-dialog.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

Given that we have so many kinds of different dialogs where would you propose to get those added?

Also given that his dialog is based on the commonDialog we have to name it accordingly: CommonDialog.

@@ +12,5 @@
> + * Base Dialog class
> + *
> + * @constructor
> + * @param {MozMillController} [aController=mozmill.getBrowserController()]
> + *        MozMillController of the Browser

Maybe adding a reference to the following MDN page:
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/dialog

@@ +44,5 @@
> +                          : this._controller.window.document;
> +  var nodeCollector = new domUtils.nodeCollector(root.documentElement);
> +
> +  switch (aSpec.type) {
> +    case "accept":

Shouldn't we use the Button suffix for all of those entries or bundle them under `button_xyz` to have them all beside each other?

Also I would add helper methods for the most used buttons like .accept();

@@ +45,5 @@
> +  var nodeCollector = new domUtils.nodeCollector(root.documentElement);
> +
> +  switch (aSpec.type) {
> +    case "accept":
> +      nodeCollector.queryAnonymousNode("dlgtype", "accept");

Hurray for getting rid of the lookup strings!

@@ +76,5 @@
> +      elems = [findElement.ID(root, "info.body")];
> +      break;
> +    case "info_icon":
> +      elems = [findElement.ID(root, "info.body")];
> +      break;

Typing error. This should be info.icon.

@@ +78,5 @@
> +    case "info_icon":
> +      elems = [findElement.ID(root, "info.body")];
> +      break;
> +    case "info_title":
> +      elems = [findElement.ID(root, "info.title")];

On OS X this is not hidden, but for all the other platforms. Until now we grabbed that via aController.window.document.title. I still think we should grab it from the title directly if available, and only fallback to this element on OS X.

Maybe we add a title property to the class to make this distinction.

Comment 6

[Cosmin Malutan, [:cosmin-malutan]]

(In reply to Henrik Skupin (:whimboo) from comment #5)
> 
> This patch looks great, Cosmin! There are some things to clarify and fix,
> but that should be done kinda quick. Also I wonder if those were already all
> the existing tests we have used the commondialog.
I didn't obsolete the first patch so you reviewed the first one some of the requests have already been fixed.


> > -  _handleEngineInstall : function engineManager_handleEngineInstall(aController) {
> > -    var confirmTitle = utils.getProperty("chrome://global/locale/search/search.properties",
> > -                                         "addEngineConfirmTitle");
> 
> As you can see this is a special dialog, and needs its own class.
It's still a common dialog, it has the same elements, just that it has different values inside of info.title and info.body elements.

> > +    var dialog = baseDialog.open(aAddEngineCallback);
> 
> BaseDialog is not going to replace any custom dialog. Here we would need a
> specific implementation. So this needs to be reverted.
> 
> Also is this not a modal dialog anymore? I ask because you completely
> removed the handling code.
It's crystal clear that it won't replace any custom dialog, but this one is just a plain common dialog, it doesn't have any extra elements that the baseDialog class has. 

> > + */
> > +BrowserWindow.prototype.openBaseDialog = function BW_openBaseDialog(aCallback) {
> > +  return baseDialog.open(aCallback);
> 
> I do not see what the idea of this method is. If there is a method which
> opens a dialog, we will need custom code.
I added this in the first stage of patch, but now I incline to think that we don't need this opening method and it would be best to compleatly remove it, and just plainly pass in the controller in class. This way we don't care about the handling of the window, and we could safely use the modal-dialog.js code that we already have.

> :::
> firefox/tests/functional/restartTests/testPreferences_masterPassword/test2.js
> @@ +108,3 @@
> >  
> > +  var passwordBox = dialog.getElement({type: "password_textbox"});
> > +  passwordBox.waitForElement();
> 
> Do we still have to wait here? Maybe we need another event specifically for
> the commondialog to wait for its completed load.
We don't need to wait, and we don't need an extra check as we wait for for state when we open the dialog. 

> 
> ::: lib/ui/base-dialog.js
> @@ +1,1 @@
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> 
> Given that we have so many kinds of different dialogs where would you
> propose to get those added?
> 
> Also given that his dialog is based on the commonDialog we have to name it
> accordingly: CommonDialog.
I would suggest in firefox/lib/ui/dialogs/

> @@ +78,5 @@
> > +    case "info_icon":
> > +      elems = [findElement.ID(root, "info.body")];
> > +      break;
> > +    case "info_title":
> > +      elems = [findElement.ID(root, "info.title")];
> 
> On OS X this is not hidden, but for all the other platforms. Until now we
> grabbed that via aController.window.document.title. I still think we should
> grab it from the title directly if available, and only fallback to this
> element on OS X.
> 
> Maybe we add a title property to the class to make this distinction.
From what I checked we have the element on all platforms, I suggest we keep the case in getElements and add a title property which will grab document.title and fallback to the element if that is null/undefined.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Cosmin Malutan from comment #6)
> > > -  _handleEngineInstall : function engineManager_handleEngineInstall(aController) {
> > > -    var confirmTitle = utils.getProperty("chrome://global/locale/search/search.properties",
> > > -                                         "addEngineConfirmTitle");
> > 
> > As you can see this is a special dialog, and needs its own class.
> It's still a common dialog, it has the same elements, just that it has
> different values inside of info.title and info.body elements.

I see now that http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#1475 modifies those elements. The only thing why I want to see a new class here is that for this dialog different localized strings are used. Those we have to hide inside the class. So it would be a really simple subclass.

> > > +    var dialog = baseDialog.open(aAddEngineCallback);
> > 
> > BaseDialog is not going to replace any custom dialog. Here we would need a
> > specific implementation. So this needs to be reverted.
> > 
> > Also is this not a modal dialog anymore? I ask because you completely
> > removed the handling code.
> It's crystal clear that it won't replace any custom dialog, but this one is
> just a plain common dialog, it doesn't have any extra elements that the
> baseDialog class has. 

As mentioned above thats clar now, but please also answer my question regarding the modal dialog.

> > ::: lib/ui/base-dialog.js
> > @@ +1,1 @@
> > > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > 
> > Given that we have so many kinds of different dialogs where would you
> > propose to get those added?
> > 
> > Also given that his dialog is based on the commonDialog we have to name it
> > accordingly: CommonDialog.
> I would suggest in firefox/lib/ui/dialogs/

I would not create a new file for each of the dialogs. I still think having a single file like firefox/lib/ui/dialogs.js should be enough. The base dialog and all shared dialogs could end-up in /lib/ui/dialogs.js?

> > On OS X this is not hidden, but for all the other platforms. Until now we
> > grabbed that via aController.window.document.title. I still think we should
> > grab it from the title directly if available, and only fallback to this
> > element on OS X.
> > 
> > Maybe we add a title property to the class to make this distinction.
> From what I checked we have the element on all platforms, I suggest we keep
> the case in getElements and add a title property which will grab
> document.title and fallback to the element if that is null/undefined.

That's what I proposed so lets do it then.

Comment 8

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v3.0

(In reply to Henrik Skupin (:whimboo) from comment #7)
> > > > +    var dialog = baseDialog.open(aAddEngineCallback);
> > > 
> > > BaseDialog is not going to replace any custom dialog. Here we would need a
> > > specific implementation. So this needs to be reverted.
> > > 
> > > Also is this not a modal dialog anymore? I ask because you completely
> > > removed the handling code.
> > It's crystal clear that it won't replace any custom dialog, but this one is
> > just a plain common dialog, it doesn't have any extra elements that the
> > baseDialog class has. 
> 
> As mentioned above thats clar now, but please also answer my question
> regarding the modal dialog.
That was a mistake, I reverted it, I just pass in the controller now.

Comment 9

[Cosmin Malutan, [:cosmin-malutan]]

http://mozmill-crowd.blargon7.com/#/functional/report/c0d2f578c4e131660e84b4e0e20f1582
http://mozmill-crowd.blargon7.com/#/remote/report/c0d2f578c4e131660e84b4e0e20ef919

Comment 10

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8547556 [details] [diff] [review]
patch v3.0

Review of attachment 8547556 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think this applies anymore as we moved testPreferences_masterPassword in testAddons folder, under a single file.

Are these something we could update too? xpinstallconfirm is used in several discovery tests.
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/testFormManager/testClearFormHistory.js#l84
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/testPreferences/testPreferredLanguage.js#l114
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/testSecurity/testMD5HashSignature.js#l105
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/restartTests/testDiscoveryPane_FeaturedModule/test1.js
http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/restartTests/testDiscoveryPane_FirstTimeModule/test1.js#l89

::: firefox/lib/search.js
@@ +9,5 @@
>  
>  Cu.import("resource://gre/modules/Services.jsm");
>  
>  // Include required modules
>  var { assert, expect } = require("../../lib/assertions");

This can be removed.

@@ +10,5 @@
>  Cu.import("resource://gre/modules/Services.jsm");
>  
>  // Include required modules
>  var { assert, expect } = require("../../lib/assertions");
> +var dialogs = require("../../lib/ui/dialogs");

This is ui, please move it in a separate block, with widgets.

::: firefox/tests/functional/restartTests/testPreferences_masterPassword/test1.js
@@ +5,5 @@
>  "use strict";
>  
>  // Include required modules
>  var { expect } = require("../../../../../lib/assertions");
> +var dialogs = require("../../../../../lib/ui/dialogs");

Same here

::: firefox/tests/remote/testSecurity/testSubmitUnencryptedInfoWarning.js
@@ +5,5 @@
>  "use strict";
>  
>  // Include necessary modules
>  var {expect} = require("../../../../lib/assertions");
> +var dialogs = require("../../../../lib/ui/dialogs");

Same here.

::: lib/tests/testCommonDialog.js
@@ +5,5 @@
> +"use strict";
> +
> +// Include required modules
> +var dialogs = require("../ui/dialogs");
> +var browser = require("../../firefox/lib/ui/browser");

Please separate.

::: lib/ui/dialogs.js
@@ +5,5 @@
> +"use strict";
> +
> +var baseWindow = require("base-window");
> +var domUtils = require("../dom-utils");
> +var windows = require("../windows");

Last 2 should be first, then separate baseWindows as ui.

@@ +83,5 @@
> +    case "button_disclosure":
> +      nodeCollector.queryAnonymousNode("dlgtype", "disclosure");
> +      elems = nodeCollector.elements;
> +      break;
> +    case "button_extra1":

What are these extra1 and 2 buttons? Can we give some more suggestive names?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Andreea Matei [:AndreeaMatei] from comment #10)
> @@ +83,5 @@
> > +    case "button_disclosure":
> > +      nodeCollector.queryAnonymousNode("dlgtype", "disclosure");
> > +      elems = nodeCollector.elements;
> > +      break;
> > +    case "button_extra1":
> 
> What are these extra1 and 2 buttons? Can we give some more suggestive names?

I would keep those names for the common dialog. Those buttons are configurable and we will never have an accurate name for them.

Comment 12

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v3.1

(In reply to Andreea Matei [:AndreeaMatei] from comment #10)
> I don't think this applies anymore as we moved
> testPreferences_masterPassword in testAddons folder, under a single file.
Why have this been moved under testAddons, it should have been moved under testPasswordManager or at least testPreferences, this test has nothing to do with addons. 

> Are these something we could update too? xpinstallconfirm is used in several
> discovery tests.
> http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/
> testFormManager/testClearFormHistory.js#l84
> http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/functional/
> testPreferences/testPreferredLanguage.js#l114
> http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/
> testSecurity/testMD5HashSignature.js#l105
> http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/
> restartTests/testDiscoveryPane_FeaturedModule/test1.js
> http://hg.mozilla.org/qa/mozmill-tests/file/default/firefox/tests/remote/
> restartTests/testDiscoveryPane_FirstTimeModule/test1.js#l89
Those are not using the CommonDialog class so we would need to extended classes, I covered the ones that used the base/CommonDialog class, but we should definitely address those in an different bug.  

> ::: firefox/lib/search.js
> @@ +9,5 @@
> >  
> >  Cu.import("resource://gre/modules/Services.jsm");
> >  
> >  // Include required modules
> >  var { assert, expect } = require("../../lib/assertions");
> 
> This can be removed.
Yes, we have bug 989297 for that.





Report:
http://mozmill-crowd.blargon7.com/#/functional/report/569158a0e5d1513e2dbac88f33f300a4

Comment 13

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8549430 [details] [diff] [review]
patch v3.1

Review of attachment 8549430 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. Lets also file a bug for those other dialogs in the tests I mentioned, so we don't forget.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8549430 [details] [diff] [review]
patch v3.1

Review of attachment 8549430 [details] [diff] [review]:
-----------------------------------------------------------------

Looks wonderful. There are some changes left to do. There is no need to re-request for review from me. Let Andreea review it.

::: firefox/lib/search.js
@@ +184,5 @@
>    _handleEngineInstall : function engineManager_handleEngineInstall(aController) {
> +    var dialog = new dialogs.AddEngineConfirmDialog(aController);
> +
> +    var title = dialog.getProperty("addEngineConfirmTitle");
> +    expect.equal(dialog.title, title, "Window contains search engine title");

Can all be a single block.

::: lib/tests/testCommonDialog.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

Please fix the filename.

@@ +33,5 @@
> +
> +  TEST_ELEMENTS.forEach((aElement) => {
> +    var element = dialog.getElement({type: aElement});
> +    expect.ok(element && element.exists(),
> +              "Element " + aElement + " has been found.");

We usually test here for .localName. Please see all the other ui modules.

::: lib/ui/dialogs.js
@@ +136,5 @@
> + *        MozMillController of the Browser
> + */
> +function AddEngineConfirmDialog(aController) {
> +  CommonDialog.call(this, aController);
> +  this._properties = ["chrome://global/locale/search/search.properties"];

Is this dialog in Toolkit? If not we would have to create another dialogs module under Firefox.

Comment 15

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v3.2

(In reply to Henrik Skupin (:whimboo) from comment #14)
> Comment on attachment 8549430 [details] [diff] [review]
> patch v3.1
> 
> Review of attachment 8549430 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks wonderful. There are some changes left to do. There is no need to
> re-request for review from me. Let Andreea review it.
> 
> ::: firefox/lib/search.js
> @@ +184,5 @@
> >    _handleEngineInstall : function engineManager_handleEngineInstall(aController) {
> > +    var dialog = new dialogs.AddEngineConfirmDialog(aController);
> > +
> > +    var title = dialog.getProperty("addEngineConfirmTitle");
> > +    expect.equal(dialog.title, title, "Window contains search engine title");
> 
> Can all be a single block.
> 
> ::: lib/tests/testCommonDialog.js
> @@ +1,1 @@
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> 
> Please fix the filename.
> 
> @@ +33,5 @@
> > +
> > +  TEST_ELEMENTS.forEach((aElement) => {
> > +    var element = dialog.getElement({type: aElement});
> > +    expect.ok(element && element.exists(),
> > +              "Element " + aElement + " has been found.");
> 
> We usually test here for .localName. Please see all the other ui modules.
> 
> ::: lib/ui/dialogs.js
> @@ +136,5 @@
> > + *        MozMillController of the Browser
> > + */
> > +function AddEngineConfirmDialog(aController) {
> > +  CommonDialog.call(this, aController);
> > +  this._properties = ["chrome://global/locale/search/search.properties"];
> 
> Is this dialog in Toolkit? If not we would have to create another dialogs
> module under Firefox.
Yes, this is under the toolkit, but as you say for other modals in browser directory we should create a new module in mozmill-tests/firefox/lib/ui

Comment 16

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8551341 [details] [diff] [review]
patch v3.2

Review of attachment 8551341 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/tests/testCommonDialog.js
@@ +11,5 @@
> +var browser = require("../../firefox/lib/ui/browser");
> +
> +const TEST_ELEMENTS = {
> +  button_accept:"button",
> +  button_cancel:"button",

These 2 don't have whitespace before.

Comment 17

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v4.0

Done, I also had an conflict.

Comment 18

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8552247 [details] [diff] [review]
patch v4.0

Review of attachment 8552247 [details] [diff] [review]:
-----------------------------------------------------------------

http://hg.mozilla.org/qa/mozmill-tests/rev/be404c433260 (default)

Comment 19

[Cosmin Malutan, [:cosmin-malutan]]

Andreea, could you backport this to aurora?

Comment 20

[Andreea Matei [:AndreeaMatei]]

https://hg.mozilla.org/qa/mozmill-tests/rev/c4d75f3d7853 (aurora)

Lets do beta too so we make sure future tests can use this.

Comment 21

[Andreea Matei [:AndreeaMatei]]

Actually, let's ride the trains on this, we have bug 1109026 only down to aurora, there are different conflicts. This is not blocking anything at the moment.