443584 - Automate litmus test Testcase ID #4572 - Queued / "Starting ..." downloads: cancel context menu

Status: ASSIGNED

Description

[Anoop Saldanha (Poona)]

Proposed type of test: chrome

Proposed location of test: toolkit/mozapps/downloads/tests/chrome

Supply dummy data to the dm's db for this test.

Comment 1

[Anoop Saldanha (Poona)]

Created attachment 328135 [details] [diff] [review]
v1.0

Actually this test requires me to check if a queued download's context has a cancel.  

To implement this, I generated a "context-click c" on the queued download and checked if DOWNLOAD_CANCELED is called. The fact that DOWNLOAD_CANCELED is called, is an indication that cancel is present in the context menu, because if cancel isn't present, then "context-click c" wouldn't have canceled the download.

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 328135 [details] [diff] [review]
v1.0

general nit: s/var/let/ please
general nit: this file has tabs in it.  Please remove them.
general nit: you have lots of trailing spaces in this file.  Please remove them.

>+ /** This is an automated version of the litmus test, written for bug 443584.
>+  *  Basically this test checks if the conext menu of a queued download
>+  *  has cancel within it.
>+  */
nit: formatting should be like this:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_basic_functionality.xul#40
nit: "context"

>+<window title="Download Manager UI Test Code"
>+  onload="setTimeout(runTest, 0);"
>+  xmlns:html="http://www.w3.org/1999/xhtml"
>+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
why the timeout?
nit: line up attributes like so:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_basic_functionality.xul#47


Actually, this file suffers from lots of the same things mentioned in bug 443585 comment 2.  Please fix and make a new attachment.

Comment 3

[Anoop Saldanha (Poona)]

Created attachment 331161 [details] [diff] [review]
v2.0

Tested - Passes

Comment 4

[Anoop Saldanha (Poona)]

Created attachment 331481 [details] [diff] [review]
v3.0

Test updated to accommodate the new utils.js.  The test passes

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 331481 [details] [diff] [review]
v3.0

>+ /** This is an automated version of the litmus test, written for bug 443584.
>+  *  Basically this test checks if the conext menu of a queued download
>+  *  has cancel within it.
>+  */
nit: formatting isn't in the normal form

>+var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
>+var dmFile = Cc["@mozilla.org/file/directory_service;1"].
>+             getService(Ci.nsIProperties).get("TmpD", Ci.nsIFile);
>+dmFile.append("dm-ui-test.file");
>+dmFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0666);
>+var gTestPath = ios.newFileURI(dmFile).spec;
not needed

>+function runTest()
>+{
>+  const dmui = Cc["@mozilla.org/download-manager-ui;1"].
>+             getService(Ci.nsIDownloadManagerUI);
>+  const dm = Cc["@mozilla.org/download-manager;1"].
>+             getService(Ci.nsIDownloadManager);
don't make these const - move them to just before first use

>+  // Close the DM UI if it is already open
>+  let win = getDMWindow();
>+  if (win) win.close();
setCleanState()

>+      let testResult = false;
>+      for(let i = 0; i < menuitem.length; i++)
>+        testResult = testResult || (menuitem[i].label == "Cancel");
this will fail on non en-us builds.  Also, no need for or - just add !testResult in the for conditional.

>+      ok(testResult,"The queued download has a Cancel in its Contextmenu");
nit: space after comma

Comment 6

[Anoop Saldanha (Poona)]

Created attachment 335249 [details] [diff] [review]
v4.0

>+var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
>>+var dmFile = Cc["@mozilla.org/file/directory_service;1"].
>>+             getService(Ci.nsIProperties).get("TmpD", Ci.nsIFile);
>>+dmFile.append("dm-ui-test.file");
>>+dmFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0666);
>>+var gTestPath = ios.newFileURI(dmFile).spec;
>not needed

Without this it isn't working for me.

Another thing.  The properties file download.properties didn't have an exclusie string for context menu's cancel.  So I retrieved the first word for this porperty - quitCancelDownloadsAlertTitle=Cancel All Downloads?

which will be Cancel and I am using this word here for comparison.  I am not sure if the first word of this property represents the word "Cancel" in every language build of FF.

let cancelString = sb.getString("quitCancelDownloadsAlertTitle");
cancelString = cancelString.substring(0, cancelString.indexOf(" "));

for(let i = 0; i < menuitem.length && !testResult; i++)
  testResult = (menuitem[i].label.indexOf(cancelString) != -1);

I have added a !testResult in the conditions list so that the loop exits as soon as it finds the "Cancel" label in the menulist.

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 335249 [details] [diff] [review]
v4.0

>+      let sb = doc.getElementById("downloadStrings");
>+      let cancelString = sb.getString("quitCancelDownloadsAlertTitle");
>+      cancelString = cancelString.substring(0, cancelString.indexOf(" "));
>+
>+      let download = doc.getElementById("downloadView").children[0];
>+      synthesizeMouse(download, 1, 1, { type: "contextmenu", button: 2 }, win);
>+
>+      let popup = doc.getElementById("downloadContextMenu");
>+      let menuitem = popup.childNodes;
>+
>+      let testResult = false;
>+      for(let i = 0; i < menuitem.length && !testResult; i++)
>+        testResult = (menuitem[i].label.indexOf(cancelString) != -1);
So, this string stuff isn't good.  Why don't you just check to make sure that menuitem[i].getAttribute("cmd") == "cmd_cancel"

Comment 8

[Anoop Saldanha (Poona)]

Created attachment 336673 [details] [diff] [review]
v5.0

Test updated with the code change suggested.

Comment 9

[Shawn Wilsher :sdwilsh]

Comment on attachment 336673 [details] [diff] [review]
v5.0

>+      obs.removeObserver(testObs, DLMGR_UI_DONE, false);
>+      setCleanState();
>+      SimpleTest.finish();
dmFile.remove(false) please before you finish

r=sdwilsh with that change