582677 - Test for dragging and dropping item from one group to another group

Status: RESOLVED FIXED

(Firefox Graveyard :: Panorama, defect)

Description

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 1

[Raymond Lee [:raymondlee]]

You can run it using:
TEST_PATH=browser/base/content/test/browser_drag_drop_tabcandy_item.js make -C obj-ff-dbg mochitest-browser-chrome

Comment 2

[Ed Lee :Mardak]

Comment on attachment 460935 [details] [diff] [review]
v1

Timers are generally bad to have in tests. It will slow down the total time to run tests and be prone to timing issues. Typically these have been avoided by listening to events for when something should have happened. Or if those events don't exist, they're added.

We'll probably want some wrappers around the drag event pattern once we figure out what behavior it should support.

Comment 3

[Raymond Lee [:raymondlee]]

(In reply to comment #2)
> Comment on attachment 460935 [details] [diff] [review]
> v1
> 
> Timers are generally bad to have in tests. It will slow down the total time to
> run tests and be prone to timing issues. Typically these have been avoided by
> listening to events for when something should have happened. Or if those events
> don't exist, they're added.
> 
> We'll probably want some wrappers around the drag event pattern once we figure
> out what behavior it should support.

Got it. Since we have zoom out/in for Tab Candy and other animations, we will need to add events to notify others when zooming out or in is actually finished for this test.

Comment 4

[Ian Gilman [:iangilman]]

(In reply to comment #3)
> Got it. Since we have zoom out/in for Tab Candy and other animations, we will
> need to add events to notify others when zooming out or in is actually finished
> for this test.

You can use the Subscribable interface that's built into Group and TabItem to create those events.

Comment 5

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 6

[Raymond Lee [:raymondlee]]

Comment on attachment 462408 [details] [diff] [review]
v1

* Removed all the timeout and using the Subscribable interface in the Group.
* Custom events are fired when tab view is shown and hidden which would also solve bug 583182

Comment 7

[Ian Gilman [:iangilman]]

Comment on attachment 462408 [details] [diff] [review]
v1

Looks great! Raymond, please apply.

I see that you've done some clean up as well... very cool. One of the benefits of writing tests! :)

Comment 8

[Ed Lee :Mardak]

Comment on attachment 462408 [details] [diff] [review]
v1

>+function simulateDragDrop(srcElement, offsetX, offsetY, contentWindow, callback) {
...
>+  callback.apply();
As mentioned last night, if you think this interface may go asynchronous, keep the callback. But as it is right now, it's all synchronous, so no need for the callback.

Comment 9

[Raymond Lee [:raymondlee]]

Applied
http://hg.mozilla.org/users/edward.lee_engineering.uiuc.edu/tabcandy-central/rev/a0ccb8c47ab8

Comment 10

[Ian Gilman [:iangilman]]

Adding ehsan as a CC, in case he wants to see the history here.

Comment 11

[Ian Gilman [:iangilman]]

This test has been temporarily disabled (in browser/base/content/test/tabview/Makefile.in), as it's failing. Please re-enable once it's fixed.

Comment 12

[(no longer active)]

Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".

Comment 13

[Raymond Lee [:raymondlee]]

Attachment: Re-enable the test with some changes

Comment 14

[Ian Gilman [:iangilman]]

Comment on attachment 467335 [details] [diff] [review]
Re-enable the test with some changes

Looks good.

Comment 15

[Justin Dolske [:Dolske]]

Comment on attachment 467335 [details] [diff] [review]
Re-enable the test with some changes

>+++ b/browser/base/content/test/tabview/Makefile.in	Thu Aug 19 15:35:55 2010 +0800
>@@ -47,8 +47,8 @@
> 
> _BROWSER_FILES = \
>                  browser_tabview_launch.js \
>-    $(NULL)
>-#                 browser_tabview_dragdrop.js \
>+                 browser_tabview_dragdrop.js \
>+$(NULL)

Fix the indenting here, $(NULL) should be aligned with everything else.

Comment 16

[Raymond Lee [:raymondlee]]

Attachment: re-enable the test with some changes [r=dolske a=dolske]

Comment 17

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/79db5d662975