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

RESOLVED FIXED in Firefox 56



11 months ago
11 months ago


(Reporter: sparky, Assigned: sparky)



Firefox Tracking Flags

(firefox56 fixed)


MozReview Requests


Submitter Diff Changes Open Issues Last Updated
Error loading review requests:


(1 attachment)



11 months ago
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:

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:

Comment 1

11 months ago
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:

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:

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:

Comment 2

11 months ago
Hi Greg, looks like you've been working on a fix for this, should I assign this bug to you?
Flags: needinfo?(gmierz2)

Comment 3

11 months ago
Hi Hsin-Yi, sounds good to me.
Flags: needinfo?(gmierz2)

Comment 4

11 months ago
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:

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:

Comment 5

11 months ago
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 hidden (mozreview-request)

Comment 8

11 months ago
Comment on attachment 8878899 [details]
Bug 1372924 - Handle input focus notifications at the right times in test_nsITextInputProcessor.xul.
Attachment #8878899 - Flags: review?(jmaher) → review+

Comment 9

11 months ago
Pushed by
Handle input focus notifications at the right times in test_nsITextInputProcessor.xul. r=jmaher

Comment 10

11 months ago
Last Resolved: 11 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.