537968 - Double click on tabstrip doesn't open a new tab when another tab has been closed before via its close button [testNewTab.js]

Status: RESOLVED WORKSFORME

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

With the new test for closing a tab (bug 536432) we have broken the test for opening a new tab. Somehow it is not possible to open a new tab when double clicking on the tab strip. I will investigate it in the next days.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Testcase

This one is really strange. The failure only happens when I use the close button of the tab before I try to open a new tab via a double click inside the tabstrip. When I use a keyboard shortcut the failure doesn't occur.

Dao, do we call the same functions when closing a tab via a shortcut and the close button? Or could this be a focus issue?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Test v2

Dao, I was investigating this problem over the last weekend at the FOSDEM and I cannot see that this could be Mozmill bug. The following items get tested by the test:

1. Opening a new tab via a dbl click on the tabstrip works at the beginning
2. Closing the new tab via its close button works
3. Opening a new tab via a dbl click on the tabstrip fails
4. Another dbl click on the tabstrip works

Steps 1 and 3 are not different. For both only one tab is open and clicking on the given position should always open a new tab. But it doesn't happen when a tab has been closed before via its close button.

Is there a problem with propagating the event to the tabstrip?

Comment 3

[Dão Gottwald [:dao]]

Can you reproduce this manually? If not, it's likely not a tabbrowser bug.

Comment 4

[Martijn Wargers (dead)]

This isn't a bug, see bug 343628. This is basically the anti-bug of that bug.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #3)
> Can you reproduce this manually? If not, it's likely not a tabbrowser bug.

Somehow the bug mail on your comment got lost. I haven't seen it until now.

It's not reproducible when I run those tests manually but given that the test only fails which this specific steps we have to find out where the problem is located. I have talked with Martijn this morning and we will try to create a more detailed test.

Comment 6

[Martijn Wargers (dead)]

Attachment: testcase (uses enhanced privileges)

Sorry, comment 4 might be wrong, not sure.

With the testcase, you see 2 methods, dblclick1 and dblclick2, where dblclick1 seems to be the thing what mozmill is doing. But it seems to me that mozmill should be firing the events as what is happening with the dblclick2 method, because that resembles reality more.

I think that would fix the failing of this test. The tabbrowser code is doing some weird stuff, where it's not only listening for dblclick, but it is also listening for a single click, where dblclick blocking can be invoked, etc. Not sure how that is working.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

In Mozmill we are using the synthesizeMouse function from EventUtils.js. As seen by Mochitests you will have to use the following calling convention to simulate a double click:

  EventUtils.synthesizeMouse(element, left, top, {clickCount: 2},
                             element.ownerDocument.defaultView);

This is the way we are doing that in Mozmill. So I wonder what's wrong and why the first double click gets eaten.

Neil, do you have an idea? Your help would be really appreciated.

Comment 8

[Martijn Wargers (dead)]

Attachment: EventUtils.js patch for mozmill

I talked to Neal and this is what needs to be changed in EventUtils.js to more reflect in reality, what happens when you do a double-click.

I'll file a new bug for EventUtils.js in mochitest:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/EventUtils.js

Comment 9

[Neil Deakin]

I think the first block (when aEvent.type is set) should remain as is. That case is intended when the caller wants to specify the exact events that get sent.

Comment 10

[Martijn Wargers (dead)]

Comment on attachment 430640 [details] [diff] [review]
EventUtils.js patch for mozmill

Sorry, this patch is not good for the clickCount 0 case.

Comment 11

[Martijn Wargers (dead)]

I filed bug 550500 for the change in EventUtils in the mozilla mochitest framework.

Comment 12

[Martijn Wargers (dead)]

Attachment: patch

This patch should be good and which is what I'm going to attach to bug 550500 too.

Comment 13

[Martijn Wargers (dead)]

Comment on attachment 430656 [details] [diff] [review]
patch

Sorry, I missed comment 9. I attached a new patch in bug 550500.
Henrik doesn't want a patch for the EventUtils.js file in MozMill anyway (he'll copy the new code himself, after it is reviewed and checked in).

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Temporary workarounds have been pushed for default and mozilla1.9.1:
http://hg.mozilla.org/qa/mozmill-tests/rev/993d24e04ad5
http://hg.mozilla.org/qa/mozmill-tests/rev/5b5df1b2b6a6

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Moving over to Mozmill. Once the underlying EventUtils bug has been fixed I will merge it with the Mozmill code.

Comment 16

[Aaron Train [:aaronmt]]

All is fine now with mozmill 1.5.1 I take it? If so, please close bug.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Last time it occurred was Oct 18th. So it has been fixed by RC1 of Mozmill 1.5.1, which means the update to the click position - we are clicking in the middle of an element per default now - has overlayed this failure.

I don't think that it is fixed for real. Aaron, can you please try by having more tabs open at the beginning of the test? IMO the test should start to fail at this point when a tab is positioned in the middle of the tab strip.

Comment 18

[Aaron Train [:aaronmt]]

I changed the test to open ten blank tabs to start and doesn't seem to have any issues on the tabStrip event type, although it has issues on the newTabButton where the test fails.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Interesting. Ok, since the failures have been gone now, lets close this bug as WFM. We can reopen if necessary.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.