444638 - Automate litmus test Testcase ID #4615 - Active download - selection order after deletion of Completed items

Status: NEW

(Toolkit :: Downloads API, defect)

Description

[Anoop Saldanha (Poona)]

Proposed type of test: Chrome

Proposed location of test: I will be adding this to an existing test - http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_delete_key_removes.xul

Comment 1

[Anoop Saldanha (Poona)]

Attachment: v1.0

Comment 2

[Anoop Saldanha (Poona)]

Attachment: v2.0

Made a small correction

Comment 3

[Anoop Saldanha (Poona)]

Attachment: v3.0

Checked.  The test works.

Comment 4

[Shawn Wilsher :sdwilsh]

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

general nit: line wrapping at 80 characters

>+/**
>+ * This tests if deleting a completed download results in the automatic seletion
>+ * of the next completed download
>+ *
>+ * @param aWin
>+ *        Holds the dm window
>+ */
no need for the params - all the tests are the same and it's obvious

>+function test_selectedNextDownload(aWin)
>+{
>+  let doc = aWin.document;
>+  let richlistbox = doc.getElementById("downloadView");
>+  let downloadToBeDeleted = 1;
>+
>+  // Select an item in the dm
>+  richlistbox.selectedIndex = downloadToBeDeleted;
>+
>+  // Get the dlid for the richlistitem of the download that is after the selected download
>+  let dlid = richlistbox.children[downloadToBeDeleted + 1].getAttribute("dlid");
>+  dlid = "dl" + dlid + " dl" + dlid;
>+
>+  // Delete the currently selected download and then check if the next completed download gets selected
>+  synthesizeKey("VK_DELETE", { }, aWin);
>+
>+  let currentSelectedDl = richlistbox.getAttribute("last-selected");
>+
>+  is(dlid, currentSelectedDl,
>+     "On deletion of a completed download, the next completed download gets selected");
>+}
Although, I don't understand why you can't add a bit more logic to the test function to accomplish what you want (and it'll check every download, not just one).

You'll also want to update http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_backspace_key_removes.xul which is the same test, but with a different key.

Comment 5

[Anoop Saldanha (Poona)]

Attachment: v4.0

Updated.  Integrated my test to work with your previous test.  Tested - passes

I haven't updated - http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/downloads/tests/chrome/test_backspace_key_removes.xul

Do you want me to create a new patch for this above file?

Comment 6

[Anoop Saldanha (Poona)]

Attachment: v5.0

I have updated test_backspace_key_removes.xul.

Surprisingly this file wasn't included in the Makefile.in.  I have added this to the Makefile.

Also backspace doesn't delete a download in ff for me!  And so the checks in test_backspace_key_removes.xul fails.

test_delete_key_removes.xul works

Both files tested and both work as expected :)

Comment 7

[Anoop Saldanha (Poona)]

Attachment: v6.0

I had a discussion at irc and they told me that backspace deletion works only in mac.  I still have included the file in the patch.

Comment 8

[Anoop Saldanha (Poona)]

Attachment: v7.0

Made a small attachment.  The test runs except for test_backspace_key_removes.xul

Comment 9

[Anoop Saldanha (Poona)]

Attachment: v8.0

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

Comment 10

[Shawn Wilsher :sdwilsh]

Comment on attachment 331486 [details] [diff] [review]
v8.0

>+        let downloadToBeDeleted = 1;
>+        for (let i = 0; i < len - 2; i++) {
>+          richlistbox.selectedIndex = downloadToBeDeleted;
>+          let dlid = richlistbox.children[downloadToBeDeleted + 1].getAttribute("dlid");
>+          dlid = "dl" + dlid + " dl" + dlid;
I don't even know what this is supposed to be doing or why.

>           is(stmt.getInt32(0), len - (i + 1),
>-             "The download was properly removed");
>+            "On pressing the delete key, the download was properly removed");
nit: s/delete/backspace/

>+          let currentSelectedDl = richlistbox.getAttribute("last-selected");
>+          is(dlid, currentSelectedDl,
>+            "On deletion of a download, the next completed download gets selected");
I still don't really know what dlid is storing, but shouldn't you just check that the selected index is reduced by one?

>+        is(stmt.getInt32(0), 0,
>+          "On pressing the delete key, the download was properly removed");
nit: s/delete/backspace/

Comment 11

[Anoop Saldanha (Poona)]

Attachment: v9.0

New patch updated

>>+        let downloadToBeDeleted = 1;
>>+        for (let i = 0; i < len - 2; i++) {
>>+          richlistbox.selectedIndex = downloadToBeDeleted;
>>+          let dlid = richlistbox.children[downloadToBeDeleted + 1].getAttribute("dlid");
>>+          dlid = "dl" + dlid + " dl" + dlid;
>I don't even know what this is supposed to be doing or why.

>>+          let currentSelectedDl = richlistbox.getAttribute("last-selected");
>>+          is(dlid, currentSelectedDl,
>>+            "On deletion of a download, the next completed download gets selected");
>I still don't really know what dlid is storing, but shouldn't you just check
>that the selected index is reduced by one?

Basically I need to check, if pressing delete/backspace results in the selection of the next completed download in the dm list.

So I retrieve the id of the current selected downlod I do -
let currentSelectedDl = richlistbox.getAttribute("last-selected");
Now this holds the dlid in the format "dl1 dl1", where 1 is the download id no.

This is the reason why I am doing this -       
let dlid = richlistbox.children[downloadToBeDeleted + 1].getAttribute("dlid");
dlid = "dl" + dlid + " dl" + dlid;

So first I retrieve the dlid of the next download in the download list and store this in dlid.  Then I delete the current selected download, and then I retrieve the current selected download and compare it with dlid to check if the next download in the list has got selected or not

Comment 12

[Shawn Wilsher :sdwilsh]

Comment on attachment 335260 [details] [diff] [review]
v9.0

(In reply to comment #11)
> >I still don't really know what dlid is storing, but shouldn't you just check
> >that the selected index is reduced by one?
> 
> Basically I need to check, if pressing delete/backspace results in the
> selection of the next completed download in the dm list.
This test is still way more complicated than it needs to be.  Why can't you test the selected index?

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.