859356 - current contextmenu dispatching doesn't work for all cases

Status: RESOLVED INVALID

(Remote Protocol :: Marionette, defect)

Description

[Malini Das [:mdas] - Away, not checking bugmail]

if I do:
actions.press(element).perform()
time.sleep(15)
actions.release().perform()

this should trigger a 'contextmenu' event, but our current code does not dispatch it in this case. The way it works now is that it checks ahead to see if we're waiting/releasing, and then calculates if it should trigger it. Instead, we should set the timer with a callback to fire contextmenu, and then, if release, cancel or a move method is called, we will set some boolean, say 'fireContextMenu', to false. It should be true by default. The timer's callback will check fireContextMenu, and if it's true, it will fire it.

Comment 1

[Ann(Yiming)]

heh I didn't see this bug until submitted bug 850819, but luckily, I found the same issue and edited long press in that patch. I did something similar in that patch: so instead of checking whether or not contextmenu should be fired in "press" block, I moved it to "wait" block so that any kind of segmented action chain will work properly.

Comment 2

[Malini Das [:mdas] - Away, not checking bugmail]

Ah, moving it to wait() won't solve for the case I have in the description. wait() is never called. We wait using time.sleep() which is outside of the action chain.

Comment 3

[Ann(Yiming)]

Hmm if we have a wait function in the action object, why do we use sleep function explicitly?

Comment 4

[Malini Das [:mdas] - Away, not checking bugmail]

we don't, but it is possible the person writing the automated test would do something like that. I think it would be reasonable to permit them to do something like this to test if a button gets highlighted and if its respective context menu appears:

actions.press().perform()
#.. do some code to check if the button is highlighted..
time.sleep(5) # really, they might have something like wait_for_element_displayed in our tests, but it does an out of action chain wait
#.. do some code to check that their context menu popped up ...
actions.release().perform()

Comment 5

[Malini Das [:mdas] - Away, not checking bugmail]

Ann brought up a good point, saying 

"to ensure button2 does show up, we may do sleep(2) between action.press(button1) and action.move(button2).release()"

and that's timing dependent, and a case where we don't want to fire contextmenu, so it seems better that we force people to use wait() when they want to fire a contextmenu, for clarity and to avoid timing issues.

Comment 6

[Malini Das [:mdas] - Away, not checking bugmail]

Any comments?

Comment 7

[David Burns :automatedtester]

I wouldnt expect

actions.press(element).perform()
time.sleep(15)
actions.release().perform()

to do a contextmenu. We have added a wait() method to the chain for this reason and if people don't use it then they get a different result.

I am definitely in agreement with Ann Yiming here because there might be valid reasons to not fire a context menu

Comment 8

[Malini Das [:mdas] - Away, not checking bugmail]

okay, sounds good. Firing a contextmenu event on wait will be resolved in the bug 850819 as mentioned above, so resolving this as invalid.