981997 - Fix test for setInputMethodActive and dead object error

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

+++ This bug was initially created as a clone of Bug #974770 +++

See the cloned bug. Before fixing the permission issue, let's fix the test and the dead object error first.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Patch v1

This is the browser-element only patch, originally intended for bug 974770.

https://tbpl.mozilla.org/?tree=Try&rev=eb8b7efe7b5f

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

The patch removed "dom.mozInputMethod.testing" pref and it runs fine locally on Linux/Firefox. The below try server run added it back to make sure all platform works without it.

https://tbpl.mozilla.org/?tree=Try&rev=1f400aabf879

Comment 3

[Yuan Xulei [:yxl] (Leave from May 18)]

Comment on attachment 8389051 [details] [diff] [review]
Patch v1

Review of attachment 8389051 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/browser-element/mochitest/browserElement_SetInputMethodActive.js
@@ +92,3 @@
>    for (let i = 0; i < 2; i++) {
> +    let frame = gFrames[i] = document.createElement('iframe');
> +    SpecialPowers.wrap(gFrames[i]).mozbrowser = true;

I guess you want to replace this |gFrames[i]| with |frame|.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

My suspicion is correct: we cannot remove the "dom.mozInputMethod.testing" pref will break ICS emulator. Since that's not the purpose of this bug, I will check-in a patch with that pref unremoved.

Also, test on Android always fail; I suspect it's a focusing issue. I propose we disable the test on Android. 

Xulei, do you agree both assessment? If so I will create the patch for check-in. Thanks for your help!

Comment 5

[Yuan Xulei [:yxl] (Leave from May 18)]

Yes, I agree with you :-) 
We have disabled mozInputMethod tests on android, so it's fine for us to disable this related test as well.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Patch for commit

Done. Let's set checkin-needed after try passes.

https://tbpl.mozilla.org/?tree=Try&rev=3f7659b20931

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Hum, it still show orange. Maybe |fail-if| does not do what documents said.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Patch for commit (Try 2)

https://tbpl.mozilla.org/?tree=Try&rev=4c29b6b80a86

s/fail-if/skip-if/

and only test Android.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Test is now correctly being skipped on Android.

Comment 10

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b37c15e6833b

Comment 11

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/mozilla-central/rev/b37c15e6833b