Closed Bug 1372924 Opened 7 years ago Closed 7 years ago

dom/base/test/chrome/test_nsITextInputProcessor.xul | runCallbackTests(aForTests=false): window.moveBy(0, 10) should cause a notification got 2, expected 1

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: sparky, Assigned: sparky)

References

Details

Attachments

(1 file)

This bug happens when we we migrate from ubuntu 12.04 to ubuntu 16.04 on mochitest-chrome. Here's a push with the failure itself: https://treeherder.mozilla.org/#/jobs?repo=try&revision=25808fe7b96cc01b646184f227870ccd3551c440&filter-searchStr=c1 It fails because two 'notify-position-change' events are received at once (or before the check). You can see this through the debugging that was done here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3ec309477df5162cfcec5267adebcf03d7a0673&filter-searchStr=c1
I believe that we are having this problem because 'callContinueTest' is true and since we are looking for only one notification, we hit this code: https://dxr.mozilla.org/mozilla-central/source/dom/base/test/chrome/window_nsITextInputProcessor.xul#3940 And this part says, "We didn't have enough notifications last time we checked, so wait for some more." However, it got the one notification that it needed already but the callContinueTest flag wasn't changed so it runs the callBack function a second time with the same notification from the looks of it. It's quite possible that this is not the case though, so I'm going to add some output and see what aTIP and TIP look like. Here's a patch with a quick-fix for this: https://hg.mozilla.org/try/rev/f0b01aa3f62aebc155761b4c3d9ba2e59d7c12f2#l1.116 It fixes the test by checking first if there are more than one notification, if there aren't reduce the expected_notifications. Otherwise, look through the notifications and make sure that we are only looking at 'notify-position-change' events, if it is anything else, it will be dumped by reducing the expected_notifications value. I'm also running a test right now to see what a passing tests output looks like: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7299cbb8679d20bd2b281e2fa00639692dc986c
Hi Greg, looks like you've been working on a fix for this, should I assign this bug to you?
Flags: needinfo?(gmierz2)
Hi Hsin-Yi, sounds good to me.
Flags: needinfo?(gmierz2)
I was wrong to assume that the callContinueTest value was causing the problem, especially since everything else works. I ran a test to see if we get any notifications before the window.moveBy call and we definitely do. Have a look at this run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93341e928ba1d3af731f950bbbf0040e42ac2124 Essentially, what seems to be happening is that we are getting a notification after the input.focus() call, and it is received/overwritten to notify-position-change after the window.moveby call. I'm trying to fix the timeout that's happening right now in this push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19ab6b0204293a3c22436f7406b5a5ffa608203f
Figured it out...the other sub-tests are interfering with the test that is failing. I removed all the other tests which run before it and found that the input wasn't focused. I added the line 'input.focus()' just before the window.moveby tests and everything worked fine after that without any notification issues. So, in short, there are way too many calls to 'input.focus()'. Another thing, this single test runs almost 16,000 sub-tests so it would be a good idea to break it down.
Assignee: nobody → gmierz2
Priority: -- → P2
Comment on attachment 8878899 [details] Bug 1372924 - Handle input focus notifications at the right times in test_nsITextInputProcessor.xul. https://reviewboard.mozilla.org/r/150138/#review155050
Attachment #8878899 - Flags: review?(jmaher) → review+
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/19ab596ae624 Handle input focus notifications at the right times in test_nsITextInputProcessor.xul. r=jmaher
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: