1239383 - Fix browser_bug537013.js and browser_zbug569342.js for e10s

Status: NEW

(Toolkit :: Find Toolbar, defect, P3)

Description

[:Felipe Gomes (needinfo for replies!)]

These two tests have findbar-related issues with e10s. Evilpie said he could take a look at these, but no promises. Tom, I'm assigning the bug to you, but if you think you won't be able to get to it, please let me know!

browser/base/content/test/general/browser_bug537013.js
browser/base/content/test/general/browser_zbug569342.js

Comment 1

[Tom S [:evilpie]]

Oh wow, this not super easy. For example this seems to fail with e10s:

  gFindBar.open();
  gFindBar._findField.focus();
  gFindBar._findField.select();
  EventUtils.sendString(aString);

Probably because the findbar doesn't open quickly enough? Seems weird though, because that is all in the chrome process.

Comment 2

[Tom S [:evilpie]]

So, I fixed this by waiting after gFindbar.open() on the "findbaropen" event. The next problem seems to be that even after calling gFindBar._findField.focus(); the findField never receives focus in e10s. Mike or Neil any ideas?

Comment 3

[Tom S [:evilpie]]

Attachment: test with annotations

Comment 4

[Neil Deakin]

The test opens four tabs and switches to them but doesn't wait for any the focus changes from that to happen, so likely the focus is being shifted to another tab.

You need to use BrowserTestUtils.switchTab to switch tabs, and other functions of BrowserTestUtils to open tabs.

Comment 5

[Tom S [:evilpie]]

Attachment: Partially working test rewrite

Thanks Neil, that was really helpful. Both tests now almost pass. 

browser_bug537013.js:
Hangs without e10s, with "Waiting for DOMContentLoaded". With e10s the test almost finishes, but fails with "New window findbar has enabled buttons!".

browser_zbug569342.js:
In e10s fails with "Find command should be disabled" twice and "Find bar should now be hidden" at the very end. I think cmd_find might not be updated with e10s.
Without e10s testFindEnabled seems to go wrong somehow. We get "Find command should not be disabled" and the test hangs with "Waiting for findbaropen".

I don't want to spend a lot of time to track down these issues.

Comment 6

[Neil Deakin]

Comment on attachment 8714848 [details] [diff] [review]
Partially working test rewrite

+  let domContentLoaded = promiseWaitForEvent(gBrowser.selectedTab, "DOMContentLoaded"); // XXX probably not right?

DOMContentLoaded fires on the child window, or bubbles up to the browser, not the tab. But might as well just use BrowserTestUtils.browserLoaded instead which should work I think.

>-    EventUtils.synthesizeKey("/", {}, gTab.linkedBrowser.contentWindow);
>+    EventUtils.synthesizeKey("/", {}, browser.contentWindow);

Definitely a red flag. The third argument should not be present (it shouldn't have been in the original code either), or, if needed, BrowserTestUtils.sendChar could be used.

Comment 7

[Tom S [:evilpie]]

Sorry, I don't plan on finishing this anytime soon.

Comment 8

[Intermittent Failures Robot]

1 failures in 908 pushes (0.001 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-central: 1

Platform breakdown:
* windows7-32: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1239383&startday=2017-08-21&endday=2017-08-27&tree=all

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 24 weeks.

Comment 10

[Handyman services Dubai]

The test opens four tabs and switches to them but doesn't wait for any focus changes from that to happen, so likely the focus is being shifted to another tab.

You need to use BrowserTestUtils.switchTab to switch tabs, and other functions of BrowserTestUtils to open tabs.
For more details or best services must visit https://handyman-services-dubai.com/