Closed Bug 582677 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox Graveyard :: Panorama, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 4.0b5

People

(Reporter: raymondlee, Assigned: raymondlee)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
No description provided.
Attachment #460935 - Flags: review?(ian)
Attachment #460935 - Flags: feedback?(edilee)
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 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.
Attachment #460935 - Flags: feedback?(edilee) → feedback-
(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.
(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.
Attached patch v1 (obsolete) — Splinter Review
Attachment #460935 - Attachment is obsolete: true
Attachment #462408 - Flags: review?(ian)
Attachment #460935 - Flags: review?(ian)
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
Blocks: 577321
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! :)
Attachment #462408 - Flags: review?(ian)
Attachment #462408 - Flags: review+
Attachment #462408 - Flags: feedback?(edilee)
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.
Attachment #462408 - Flags: feedback?(edilee) → feedback+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Adding ehsan as a CC, in case he wants to see the history here.
Depends on: 574217
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Version: unspecified → Trunk
Attachment #467335 - Flags: review?(dolske)
Attachment #467335 - Flags: feedback?(ian)
Comment on attachment 467335 [details] [diff] [review]
Re-enable the test with some changes

Looks good.
Attachment #467335 - Flags: feedback?(ian) → feedback+
QA Contact: tabcandy → tabcandy
Attachment #462408 - Attachment is obsolete: true
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.
Attachment #467335 - Flags: review?(dolske)
Attachment #467335 - Flags: review+
Attachment #467335 - Flags: approval2.0+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/79db5d662975
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b5
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.