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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: sparky, Assigned: sparky)
References
Details
Attachments
(1 file)
Bug 1372924 - Handle input focus notifications at the right times in test_nsITextInputProcessor.xul.
(deleted),
text/x-review-board-request
|
jmaher
:
review+
|
Details |
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
Assignee | ||
Comment 1•7 years 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: 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
Comment 2•7 years ago
|
||
Hi Greg, looks like you've been working on a fix for this, should I assign this bug to you?
Flags: needinfo?(gmierz2)
Assignee | ||
Comment 4•7 years 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: 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
Assignee | ||
Comment 5•7 years 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.
Updated•7 years ago
|
Assignee: nobody → gmierz2
Priority: -- → P2
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
Comment 8•7 years ago
|
||
mozreview-review |
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
Comment 10•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•