I tried to fix this, but I couldn't test this locally, because I would then run into bug 918732. I've attached the fix that I was trying. This gave on tryserver the following errors: 79 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/plugins/test/mochitest/test_cocoa_focus.html | Plugin should not have focus. - got true, expected false 80 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/plugins/test/mochitest/test_cocoa_focus.html | Focus event count should be 4 - got 3, expected 4

Comment on attachment 809160 [details] [diff] [review] cocoa.diff Review of attachment 809160 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/test/mochitest/cocoa_focus.html @@ +97,1 @@ > window.blur() removes the focus, while i'd expect SpecialPowers.focus() to do the opposite.

(In reply to Georg Fritzsche [:gfritzsche] from comment #1) > window.blur() removes the focus, while i'd expect SpecialPowers.focus() to > do the opposite. Never mind, i missed that this is intended to focus() another window.

(In reply to Georg Fritzsche [:gfritzsche] from comment #2) > (In reply to Georg Fritzsche [:gfritzsche] from comment #1) > > window.blur() removes the focus, while i'd expect SpecialPowers.focus() to > > do the opposite. > > Never mind, i missed that this is intended to focus() another window. Yes, I did that, because there is no SpecialPowers.blur(). But focusing another window should have the same effect.

Ok, so if i'm not missing something again: Why are you replacing only one (the second) occurence of window.blur() in cocoa_focus.html? I assume if you replace the first one it fails there already? The test as is depends on the focus/blur having affected the window states when they return. I think with the things SpecialPowers does this doesn't hold anymore. Maybe you just need to wait for the focus/blur event to happen? Maybe you can even confirm this in a simpler, plugin-less, test-case?

(In reply to Georg Fritzsche [:gfritzsche] from comment #4) > Ok, so if i'm not missing something again: Why are you replacing only one > (the second) occurence of window.blur() in cocoa_focus.html? I assume if you > replace the first one it fails there already? Oops, sorry, I forgot to replace the first occurence, I should have done that. I guess that might be the reason for the failures on try server.

Another issue might be that dom.disable_window_flip pref is set to true: http://mxr.mozilla.org/mozilla-central/source/browser/app/profile/firefox.js#489 I guess with enablePrivilege set, scripts can lower or raise windows as they like. With enablePrivilege removed, that pref should be set to false, temporarily, perhaps.

Pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=84f031582b74

Previous patch still gave failures. This is another try.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=bf5256b5bfff

That didn't work either.

I was able to not getting stuck with bug 918732, by adding plugin1.focus() after the sendNativeMouseEvent calls. I'm not sure if that's allowed. I guess it's not really necessary, because on try and tbpl, bug 918732 doesn't seem to occur. Anyway, with those plugin1.focus() calls, I was able to fix the failures, which results in this patch.

I know I originally wrote a large chunk of these tests. But I need to dig back into whatever documentation still exists (in the relevant bugs), plus whatever memory I can trigger of the issues I faced when I wrote them, before I'll understand the issues here. I especially need to recover why I used the particular combination of cross-platform and native events in these tests. I should be able to get to this sometime next week.

Steven, did you take a look at this?

> Steven, did you take a look at this? It's been on my list for some time ... but it keeps getting bumped by more urgent things. No, I haven't forgotten it, and I hope to get to it in the next couple of weeks.

Sorry, I hope I don't annoy you, but if you could take a look at this, at some point, that would be great. There is no rush to it, though.

Sorry Martijn :-( This bug has been on my list for months now, but keeps getting bumped by other things.

There was some talk about removal of enablePrivilege usage, lately, in bug 984012. This is one of the few tests that still use it.

Is it possible to call DOMWindowUtils.screenPixelsPerCSSPixel and DOMWindowUtils.sendNativeMouseEvent() without special privileges? The test won't work without those. Also, as best I can tell, this test works fine since my changes to it in bug 1117027. Is that right?

Oops, I think I may have misunderstood your question, Martijn. Are you referring to the following line? And are you asking whether it's possible for us to remove it? https://hg.mozilla.org/mozilla-central/annotate/b2e71f32548f/dom/plugins/test/mochitest/cocoa_focus.html#l18 If so, I'm afraid the answer is "I don't know".

> I'm afraid the answer is "I don't know". But I'm willing to try it out, if that's what you're after.

Ok, I'm not suffering from bug 918732 anymore, it seems, I was able to run this test locally with this patch applied. The specialpowers.focus stuff is needed, otherwise the test will fail.

Comment on attachment 8580744 [details] [diff] [review] 920013.diff + plugin1.focus(); + plugin2.focus(); These defeat the whole purpose of the test, as best I can tell. The test is to tell whether or not the synthesized mouse events focus the appropriate plugin.

Ok, they don't seem to be necessary in order for the mochitest to pass, anyway.

Comment on attachment 8580791 [details] [diff] [review] 920013.diff This looks fine. But I want to run it through the tryservers, just to be sure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a58cf7c90163

Comment on attachment 8580791 [details] [diff] [review] 920013.diff The tryserver tests finished (finally), and they went fine.