Closed Bug 490548 Opened 15 years ago Closed 10 years ago

A MozMill test should fail if an element we operate on is not visible

Categories

(Testing Graveyard :: Mozmill, defect, P3)

Product:
Testing Graveyard
Component:
Mozmill
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: whimboo, Unassigned)

References

Details

(Whiteboard: [mozmill-2.0-])

Attachments

(2 files)

Mozmill test
977 bytes, application/x-javascript
Details
Another Mozmill test (searching for extensions)
976 bytes, application/x-javascript
Details
Attached file Mozmill test — 
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090423 Minefield/3.6a1pre ID:20090423040020

With the given build here, we run into a problem where MozMill normally should mark the test which is attached to this bug as failed. Because of bug 489828 Minefield didn't come up with the editBookmarksPanel. But even running the tests against this panel and its elements succeed. Whether the type nor the click function failed. I have no idea what happened at this stage but running functional tests we should behave the way as the user would be able to do in-front of his computer. Succeeding by clicking on hidden elements would not be feasible. We really should mark tests as failed in such situations.

Steps:
1. Download the Minefield build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/04/2009-04-23-04-mozilla-central/
2. Execute the attached test
IMO this should be a blocker because it stops us from detecting failures in the UI.
Severity: normal → blocker
Priority: -- → P1
Another example why we should only run actions on elements which are visible, is the search for add-ons inside the add-ons manager. On OS X there is no magnifying glass on the right side for a search textfield when the searchbutton mode is active. With the current behavior we can still click on those elements and the test passes. Normally this test should fail.
Adam, any ideas on how we can check if an element is visible?
We should take care too, that the element itself can be "visible" while the parent has the display:none attribute set. I don't know if the attribute gets propagated down the tree.
Probably document.defaultView.getComputedStyle() could help us here?
We can check display and visibility to be more sure, but this still doesn't tell us if an absolute element is on top of it making it unaccessible. It also doesn't tell us if it's off the page and unaccessible.

I wonder if there is logic around somewhere that figures out if an element is available to the user because this code could get pretty big and terrible.
Marco or David, do you know a good way of doing that? Do we really have to check for everything?
Clint, this blocks in a couple of ways. Can we get this into Mozmill 1.2?
Blocks: 491927
(In reply to comment #7)
> Marco or David, do you know a good way of doing that? Do we really have to
> check for everything?

Heya. Our related C++ code for this is in nsAccessible.cpp method nsAccessible::IsVisible(PRBool *aIsOffscreen) but it sure isn't pretty.

I assume in the attached test case controller.click doesn't synthesize a mouse dom event (via something like initMouseEvent), but instead makes a direct call?
(In reply to comment #9)
> Heya. Our related C++ code for this is in nsAccessible.cpp method
> nsAccessible::IsVisible(PRBool *aIsOffscreen) but it sure isn't pretty.

Which is here:
http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessible.cpp#875

Is there a way we can get this state via JavaScript without having to implement a XPCOM component?

> I assume in the attached test case controller.click doesn't synthesize a mouse
> dom event (via something like initMouseEvent), but instead makes a direct call?

The code which is used in Mozmill right now can be found here:
http://code.google.com/p/mozmill/source/browse/trunk/mozmill/extension/resource/modules/controller.js#232
Here's a skeleton you can try:

gAccRetrieval = Components.classes["@mozilla.org/accessibleRetrieval;1"].
  getService(nsIAccessibleRetrieval);
var acc = gAccRetrieval.getAccessibleFor(someElement);
var state = {}, extraState = {};
acc.getState(state, extraState);

if (state & nsIAccessibleStates.STATE_OFFSCREEN) // problemo

Aside: I'd really like some nsIViewManager conveniences like a method to tell me if a DOM node is fully, partially, or not occluded... and a method to return me the topmost DOM node, given a screen or window coordinate. Not sure how to push this.
(In reply to comment #11)
> gAccRetrieval = Components.classes["@mozilla.org/accessibleRetrieval;1"].
>   getService(nsIAccessibleRetrieval);

Somehow that doesn't work. This class cannot be found ("Components.classes['@mozilla.org/accessibleRetrieval;1'] is undefined").

> Aside: I'd really like some nsIViewManager conveniences like a method to tell
> me if a DOM node is fully, partially, or not occluded... and a method to return
> me the topmost DOM node, given a screen or window coordinate. Not sure how to
> push this.

Boris, could you help us here of if not do you know the correct person we could ask for?
I would say do not use accessibility until you need accessibility. If all you
need is to check if element is visible then accessibility usage is not right
thing and it's bad workaround. 

If you need to know if element is hidden then you can use GetComputedStyle
If you need to know if element is offscreen then you can get its rect by
getBoundingClientRect() of nsIDOMNSElement and compare it with a client rect.
If you have scrolled elements inside client then you can go up and compare the
rect with rects of parents. Something like I think.
> This class cannot be found
> ("Components.classes['@mozilla.org/accessibleRetrieval;1'] is undefined").

You did enable accessibility when building, right?

> if a DOM node is fully, partially, or not occluded

You'd have to define precisely what you mean by that first.

> a method to return me the topmost DOM node, given a screen or window
> coordinate

https://developer.mozilla.org/En/DOM:document.elementFromPoint (though if you expect to get back Text nodes this won't quite work for you).
Anything that requires a special build is a non-starter for mozmill.

I'm sure we'll be doing some API for accessibility testing in the future that will use those, but they will need to stay seperated from mozmill proper so that you don't need special builds to write/run tests.
> Anything that requires a special build is a non-starter for mozmill.

Our nightlies enable accessibility.  A lot of developers' personal builds don't, especially on Windows....
We don't care so much about nightlies or dev builds, the tests need to run on *release* builds. 

Anything that is built in to a release build we can depend on, if it's not, we can't. If it's in the nightlies and releases but not in some dev builds I don't mind depending on it at all.
Release builds most certainly have accessibility built.
And I'd certainly hope that the tests need to be able to run in tinderbox builds, not just releases!
What i meant by "release" was the build "process" not the fact that it had been shipped. Any build that is built under the process we build releases should be able to run mozmill tests.
I can implement the following recommendation from Alexander, which should get us closer to what we need.

If you need to know if element is hidden then you can use GetComputedStyle
If you need to know if element is offscreen then you can get its rect by
getBoundingClientRect() of nsIDOMNSElement and compare it with a client rect.
If you have scrolled elements inside client then you can go up and compare the
rect with rects of parents. 

In addition to checking visibility and display we should get 90% the way there.
(In reply to comment #21)
> If you need to know if element is hidden then you can use GetComputedStyle
> 
> In addition to checking visibility and display we should get 90% the way there.

That will not help us at all. There are a lot of UI elements which exist all the time in the DOM but are not visible because the parent window/panel is hidden. The elements themselves are still visible when querying their computed style. Do we have to walk up the tree to detect if parents are hidden/collapsed? But given by the spec shouldn't this attribute be inherited?

As an example we can use the done button (editBookmarkPanelDoneButton) inside the editBookmarkPanel. It is always visible while the panel is hidden.
Any way I think walking DOM is preferable to load accessibility in this case because accessibility a heavy enough - it creates accessible tree, mutate it, fires events and etc. Probably it's worth to request to extend some existing interface like nsIDOMNSElement to meet your needs.
(In reply to comment #18)
> Release builds most certainly have accessibility built.

For Windows and Linux yes. Mac, we're working on, but Ben recently enable the a11y build here unit tests (bug 482598).
> But given by the spec shouldn't this attribute be inherited?

visibility is, yes.  Are the things being hidden using visibility?
(In reply to comment #22)
> As an example we can use the done button (editBookmarkPanelDoneButton) inside
> the editBookmarkPanel. It is always visible while the panel is hidden.

(In reply to comment #25)
> > But given by the spec shouldn't this attribute be inherited?
> 
> visibility is, yes.  Are the things being hidden using visibility?

Marco, can you help us here again? How do you hide the editBookmarkPanel?
(In reply to comment #14)
> > if a DOM node is fully, partially, or not occluded
> 
> You'd have to define precisely what you mean by that first.
> 
> > a method to return me the topmost DOM node, given a screen or window
> > coordinate
> 
> https://developer.mozilla.org/En/DOM:document.elementFromPoint (though if you
> expect to get back Text nodes this won't quite work for you).

I'm thinking more application level. If there is a menu popup for example that is draped down over the content area, and I want the screen coordinates to give me the topmost element, then if the coords are somewhere over the menu popup, then I'd like the corresponding node in the menu, not the node hidden underneath in content. I started a patch for this over in bug 480347 but got pulled away from that.
This works at least for the keypress and type functions now. We came to this state with my patch on bug 494071 which changed the above mentioned functions to use synthesizeKey from EventUtils.js for any elements and not only windows.

If an element is not visible (e.g. no search bar in navigation toolbar) and you will send an event we get the following exception:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowUtils.sendKeyEvent]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///Volumes/data/build/mozmill/trunk/mozmill/extension/resource/modules/EventUtils.js :: synthesizeKey :: line 326"  data: no]
Source File: file:///Volumes/data/build/mozmill/trunk/mozmill/extension/resource/modules/frame.js
Line: 411

Boris, is it something we should fix in EventUtils.js or is this expected?
I have no idea what the question is even asking.  synthesizeKey doesn't let you specify which element should be getting the event, so what exactly are you doing with it?
My best guess is that we are unable to define utils based on the window we pass in, so calling utils.sendKeyEvent fails, wonder if we should have some failover code to ensure that we can get a handle on utils before we call that code.
(In reply to comment #28)
> This works at least for the keypress and type functions now. We came to this
> state with my patch on bug 494071 which changed the above mentioned functions
> to use synthesizeKey from EventUtils.js for any elements and not only windows.

Sorry this assumption was wrong as it turned out this week. I tested with the search field which was removed by the customize palette. That way it get really removed from the DOM. So it doesn't work for keypress or type.
Not really a blocker. Lowering severity to major.
Severity: blocker → major
This is a serious issue as detected today while running the software update tests. See bug 523324. We really need a solution for those cases.

Neil, how do Mochitests handle this situation?
Handle what situation?

I've never understood the point of 'MozMill tests' or what exactly they are for, so you'll need to explain what the bug is here in terms I can understand.
mochitests don't provide a high level abstraction for simulator user interactions so that there isn't really a place for this.

If a mochitest interacts with an element that isn't visible and the test author doesn't write code that checks if it's visible, the test will surely pass.

Features like this only really come up when you have an API that is saying "Click this element like a user", which is what mozmill provides.
(In reply to comment #34)
> Handle what situation?

Just to give an example: Today I run the Mozmill tests for our software updates on the betatest channel and got a different behavior as compared to the manual steps. When running the update manually we accidentally get the license wizard page for a fallback update (attachment 407259 [details]). Given that the next button is not visible and users cannot click on it. But running the Mozmill test which uses a synthesizeMouse call to click the next button doesn't recognize that the button is hidden and moves the wizard to the next step which causes to download the build. That should not happen from a user perspective.

Given your question I assume that are similar problems with Mochitests?
Depends on: 579612
Way long outstanding bug we should really fix. The current behavior makes Mozmill only half-wise helpful.

The beginning we have here:
http://hg.mozilla.org/qa/mozmill-tests/file/default/lib/utils.js#l161
Whiteboard: [mozmill-2.0?]
Whiteboard: [mozmill-2.0?] → [mozmill-2.0-][mozmill-next?]
Priority: P1 → P3
Mozmill will reach its end of life soon. We are currently working on getting all the tests for Firefox ported to Marionette. For status updates please see bug 1080766.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Whiteboard: [mozmill-2.0-][mozmill-next?] → [mozmill-2.0-]
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: