Closed
Bug 506275
Opened 15 years ago
Closed 15 years ago
Select doesn't behave as manual select on menulist
Categories
(Testing Graveyard :: Mozmill, defect)
Testing Graveyard
Mozmill
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: merike, Assigned: whimboo)
Details
(Whiteboard: [verified-mozmill-1.2.1])
Attachments
(2 files, 2 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
cmtalbert
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; et; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)
Build Identifier:
Current version of select appears to be working but it doesn't trigger the same behaviour as manual select does. Even though right option can be seen selected in event dialog in calendar status change to "needs action" should also activate percent complete textbox next to it, but currently it doesn't.
Reproducible: Always
Reporter | ||
Comment 1•15 years ago
|
||
Attachment #390473 -
Flags: review?(ctalbert)
Comment on attachment 390473 [details]
Patch to fix issue
This seems like a good idea, but when I run this with the eventsdialogv2 test from bug 500469, I still get the popups that never go away. So, maybe this isn't all we need to do.
Attachment #390473 -
Flags: review?(ctalbert) → review-
Here is a copy of the eventsdialog version 2 test. I did a little more hacking on this, and found that putting in the sleeps after the clicks helped a lot. Running this test now on a build of mozmill with the patch from attachment 390473 [details] in place does close most of the menu popups. The only menupopup I'm not seeing close is the "Reminder" menu popup which occurs just before the attendees dialog is launched.
Because the attendee dialog causes a hang on my computer, I tried commenting it out and running the test, but even doing that the "reminders" menu popup is still not closed after making the selection.
I'm really stumped as to what is making these menu popups remain open like this. Mikeal or Adam, do you have any ideas?
Merike's patch above does seem to help. If I run without the patch, all the menu popups remain displayed. So, the patch above is a step in the right direction, but it's not all the way there yet. These sleeps certainly seem to help matters a little bit.
(In reply to comment #3)
> Merike's patch above does seem to help. If I run without the patch, all the
> menu popups remain displayed. So, the patch above is a step in the right
> direction, but it's not all the way there yet. These sleeps certainly seem to
> help matters a little bit.
Hmmm actually it's not quite that easy.
* No patch above, no sleeps --> no menu popups are ever closed
* With patch above, no sleeps --> all popups except reminder list are closed
* No patch above, with sleeps --> all popups except reminder list are closed
* With patch above and sleeps --> all popups except reminder list are closed
Is there just something wrong with the reminder list?
Commenting out reminder list and the attendee dialog (which hangs my test no matter what) I get the following:
* no patch, no sleeps --> all popups displayed
* no patch, with sleeps --> the "privacy" menupopup (the next menu popup following the reminder popup in the test is not closed.
I'm at a loss. I don't think it is anything specific to do with the reminder list, and I think it must have something to do with the way that we are not handling popups properly, but I have no idea why my sleep idea works just as well as Merike's patch (which makes a whole lot more sense), and why both of them together produce exactly the same behavior as either of them applied separately.
For reference, my "sleeps" that I'm referring to are the event.sleep(xx) calls occurring at lines: 136, 143, 145, and 151 (the one at 151 was trying to get attendee dialog to work, you can ignore that one) in the eventdialogv2 test attached above.
Reporter | ||
Comment 5•15 years ago
|
||
Event dialog test has 0 controller.select calls.
Please try the patch again with this testcase.
Assignee | ||
Comment 6•15 years ago
|
||
Clint, I believe this is related to bug 504468. Looks like we should find a solution with the sleep calls at all. Adding as dependency?
Reporter | ||
Comment 7•15 years ago
|
||
Pinging about comment #5.
I'd really like to use controller.select in testcases and not a suspicious click on a menuitem which is essentially the same as the patch but doesn't really reflect that it's selecting from a list and not just clicking a regular element.
Resolving bug 504468 would allow to improve the select call in future so that it also clicks on a menulist before clicking on a menuitem. That would be closer to what user would do as it would first open dropdown and then select an option. But for now I'd take a select call that triggers the same functionality as manual click even though it's not the same visually.
One more note: popup not closing is triggered when there's two clicks, if there's just the one on a menuitem then it's not a problem.
Assignee | ||
Comment 8•15 years ago
|
||
Ok, the whole implementation of the select method was a bit wacky. There are a couple of situations where the function fails. I did mostly a rewrite now. The only thing which doesn't work is the synthesizeMouse call for content select elements. So I use .selected = true at the moment. All other things and also the popup closing should work.
Assignee: nobody → hskupin
Attachment #390473 -
Attachment is obsolete: true
Attachment #390589 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #394049 -
Flags: review?(ctalbert)
Comment on attachment 394049 [details] [diff] [review]
Patch v1
Sorry it took so long to get this review done. This looks good, and I tested it with Merike's infamous event dialog patch on Thunderbird/Lightning and all the popups closed properly.
The only comment I have here is that it now appears that we have a special behavior on this function if you pass in -1 for the index. Could you please document that on MDC in the select section?
Very nicely done, Henrik! r=ctalbert
Attachment #394049 -
Flags: review?(ctalbert) → review+
Comment 10•15 years ago
|
||
Oh and I forgot to mention, this might make bug 504468 unnecessary. We'll have to test around and see if bug 504468 is still needed.
Sorry for the spam.
Comment 11•15 years ago
|
||
r541
Thanks!!!
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•15 years ago
|
||
Docs have been updated on https://developer.mozilla.org/en/Mozmill_Tests/Mozmill_Controller_Object. I have also fixed some other mistakes like using 0 for a not used parameter. That should be null.
Bug 504468 is still needed. The way we are going with this test is just a workaround. The sleep call has also to be done for other mouse/keypress events. I'll check how we can do that.
Further I have filed bug 510519 to get multiple selections work with Mozmill.
Assignee | ||
Comment 13•15 years ago
|
||
Looks fine for existing tests. Marking as verified.
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•15 years ago
|
Whiteboard: [verified-mozmill-1.2.1]
Assignee | ||
Comment 14•15 years ago
|
||
An additional fix went in to allow closing the popup after the selection has been made:
http://github.com/mikeal/mozmill/commit/03c31c2c8d56461702f6385939b93ca9fbc3e0c9
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•