Closed Bug 981997 Opened 6 years ago Closed 6 years ago

Fix test for setInputMethodActive and dead object error

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: timdream, Assigned: timdream)

References

Details

Attachments

(1 file, 2 obsolete files)

+++ 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.
Attached patch Patch v1 (obsolete) — Splinter Review
This is the browser-element only patch, originally intended for bug 974770.

https://tbpl.mozilla.org/?tree=Try&rev=eb8b7efe7b5f
Attachment #8389051 - Flags: review?(xyuan)
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 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|.
Attachment #8389051 - Flags: review?(xyuan) → review+
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!
Flags: needinfo?(xyuan)
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.
Flags: needinfo?(xyuan)
Attached patch Patch for commit (obsolete) — Splinter Review
Done. Let's set checkin-needed after try passes.

https://tbpl.mozilla.org/?tree=Try&rev=3f7659b20931
Attachment #8389051 - Attachment is obsolete: true
Attachment #8389576 - Flags: review+
Hum, it still show orange. Maybe |fail-if| does not do what documents said.
https://tbpl.mozilla.org/?tree=Try&rev=4c29b6b80a86

s/fail-if/skip-if/

and only test Android.
Attachment #8389576 - Attachment is obsolete: true
Attachment #8389671 - Flags: review+
Test is now correctly being skipped on Android.
Keywords: checkin-needed
Attachment #8389576 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/b37c15e6833b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.