413985 - Space key for pause/resume doesn't work (never did)

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Simon Bünzli]

Comment 1

[Simon Bünzli]

Attachment: three one-line fixes

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 299197 [details] [diff] [review]
three one-line fixes

ug, I missed this one too :(

We are going to need a test for this to land as well (per toolkit policy).  Are you up for that?

Comment 3

[Simon Bünzli]

That looks like an awful lot of code is needed to test this 3 line change (for which tests should already exist in the first place). :-( Anyway, I currently haven't got a build environment, so I couldn't test the tests even if I felt like writing them. Sorry.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 299197 [details] [diff] [review]
three one-line fixes

a=beltzner for 1.9

Comment 5

[Shawn Wilsher :sdwilsh]

Thanks!  Working on a test for this now (download tests are hard :( )

Checking in toolkit/mozapps/downloads/content/downloads.js;
new revision: 1.134; previous revision: 1.133
Checking in toolkit/mozapps/downloads/content/downloads.xul;
new revision: 1.46; previous revision: 1.45

Comment 6

[Shawn Wilsher :sdwilsh]

Attachment: v1.0 test

Test.  This was a royal pain in the rear... (see bug 414171)

Comment 7

[Shawn Wilsher :sdwilsh]

Note, this test cannot land until bug 411521 is resolved.

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

https://litmus.mozilla.org/show_test.cgi?id=4552 (for pause case)
https://litmus.mozilla.org/show_test.cgi?id=4577 (for resume case)

I know that these two testcases share some overlap, since in order to resume you must have paused, but looking at how large each is by itself, combining them would be too much.

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008012704 Minefield/3.0b3pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012704 Minefield/3.0b3pre

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3pre) Gecko/2008012703 Minefield/3.0b3pre

Comment 10

[Shawn Wilsher :sdwilsh]

Stephen - I hope you didn't write those!  I flagged this as ? because we'll have automated tests landing for this soon (you can get rid of the manual ones)

Comment 11

[Shawn Wilsher :sdwilsh]

setting dependency for my own sanity to know when to land the tests.

Comment 12

[Shawn Wilsher :sdwilsh]

Checking in toolkit/mozapps/downloads/tests/browser/Makefile.in;
new revision: 1.9; previous revision: 1.8
Checking in toolkit/mozapps/downloads/tests/browser/browser_bug_413985.js;
initial revision: 1.1

Comment 13

[Simon Bünzli]

Thanks for the test, Shawn!

Comment 14

[[e-mail]]

i disabled two of these tests since at least one of them has been randomly failing on qm-centos5-01.

Checking in Makefile.in;
/cvsroot/mozilla/toolkit/components/places/tests/browser/Makefile.in,v  <--  Makefile.in
new revision: 1.2; previous revision: 1.1
done

Comment 15

[Shawn Wilsher :sdwilsh]

Files improperly named - commented in right bug.

Comment 16

[Peter Van der Beken [:peterv]]

(In reply to comment #15)
> Files improperly named - commented in right bug.

Which bug would that be? browser_bug413985-httprefresh.js seems to fail randomly too.

Comment 17

[Kai Engert [:KaiE:]]

Should we disable browser_bug413985-httprefresh.js too?
I see orange on qm-centos5-01 and apprently there was no related check-in.

Comment 18

[Shawn Wilsher :sdwilsh]

Bug 416066.  It might be best to disable them all for the time being, as they are all structured the same.  The fun part is going to be how to write them so that they are sane (the C++ code uses nsITimers, making testing difficult on the tinderboxen)