Closed Bug 938242 Opened 7 years ago Closed 7 years ago

remove visibility check in Actions

Categories

(Testing :: Marionette, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox26 wontfix, firefox27 wontfix, firefox28 fixed, b2g-v1.2 fixed)

RESOLVED FIXED
mozilla28
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- fixed
b2g-v1.2 --- fixed

People

(Reporter: etienne, Assigned: automatedtester)

References

Details

Attachments

(1 file)

In bug 936585 we're testing edge gestures that are initiated on "invisible" panels on each side of the screen.

We have to enable the debug mode during the test (which gives the panels some opacity) in order to cover this with marionette tests. Otherwise we get a:

 ElementNotVisible: (11) Element is not currently visible and may not be manipulated

I wonder if there is a strong enough use case to add an option allowing the manipulation of invisible elements.
You could try touching on the top-most visible element but use pixel references to target the actual invisible panel section.

As the touch events come in at native/Gecko level it should work but you never know, it might not.
It works exactly as I described above in comment #1.
Here we have done it with Python tests
https://github.com/mozilla-b2g/gaia/pull/13697/files

However it does not work on desktopb2g owing to bug #891882
Is Actions not good enough for this? Since they should be "do what I say" type API and should ignore visibility.
(In reply to David Burns :automatedtester from comment #3)
> Is Actions not good enough for this? Since they should be "do what I say"
> type API and should ignore visibility.

Actions check the visibility of the element you're pressing on: https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#941
(In reply to Malini Das [:mdas] from comment #4) 
> Actions check the visibility of the element you're pressing on:
> https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-
> listener.js#941

The actions interfaces are meant to call down to the underlying mouse interaction layer.  It'll calculate coordinates, so if those can't be derived there's an error.  But once those are done, the click happens at a point, not on an element.  Hence the visibility check on the “underlying” element seems wrong.
(In reply to Andreas Tolfsen (:ato) from comment #5)
> (In reply to Malini Das [:mdas] from comment #4) 
> > Actions check the visibility of the element you're pressing on:
> > https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-
> > listener.js#941
> 
> The actions interfaces are meant to call down to the underlying mouse
> interaction layer.  It'll calculate coordinates, so if those can't be
> derived there's an error.  But once those are done, the click happens at a
> point, not on an element.  Hence the visibility check on the “underlying”
> element seems wrong.

Heh, I'm not saying that it's right, I'm just saying that's the current behaviour. We can change it since that makes more sense
There is a simple workaround anyway; just like I did in the Python version linked above.

I would not like the option to manipulate invisible elements. It would be open to abuse through lazy programming, result in poor UI testing and loads of false positives.
(In reply to Andreas Tolfsen (:ato) from comment #5)
> (In reply to Malini Das [:mdas] from comment #4) 
> > Actions check the visibility of the element you're pressing on:
> > https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-
> > listener.js#941
> 
> The actions interfaces are meant to call down to the underlying mouse
> interaction layer.  It'll calculate coordinates, so if those can't be
> derived there's an error.  But once those are done, the click happens at a
> point, not on an element.  Hence the visibility check on the “underlying”
> element seems wrong.

This is correct, for actions we shouldnt call down to visibility (and I probably said it wrong in the first place, sorry). Actions is a "Do what I say" API 

For API elements hanging off webelement, so click(), send_keys() and tap() we should check visibility. WebElement API is " Do what I mean" API.

Hope that clarifies things.
(In reply to Zac C (:zac) from comment #7)
> There is a simple workaround anyway; just like I did in the Python version
> linked above.
> 
> I would not like the option to manipulate invisible elements. It would be
> open to abuse through lazy programming, result in poor UI testing and loads
> of false positives.

So we arent going too add an option, we are just doing another alignment with WebDriver. The Actions API can abuse things anyway. For times where we are using actions to get around visibility bugs I would want us to document( read as raise bug depending on the visibility bug) so that the Right Way™ is then put in place
Summary: Add an option to manipulate invisible elements → remove visibility check in Actions
Blocks: 862156
Assignee: nobody → dburns
pushed to try will check later
Comment on attachment 8339258 [details] [diff] [review]
Remove the visibility checks for touch actions in Marionette;

Review of attachment 8339258 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!
Attachment #8339258 - Flags: review?(mdas) → review+
https://hg.mozilla.org/mozilla-central/rev/234a044ca942
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Can we get this into b2g26_v1.2 as well, so we can enable the music app tests there too?
Flags: needinfo?(dburns)
Flags: needinfo?(dburns)
Whiteboard: [checkin-needed-b2g26]
You need to log in before you can comment on or make changes to this bug.