Closed Bug 932145 Opened 6 years ago Closed 6 years ago

Mochitest support for Keyboard/IME API

Categories

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: xyuan, Assigned: xyuan)

References

Details

Attachments

(1 file, 4 obsolete files)

We have implemented the Keyboard/IME API in gecko. To ensure it works properly and not isn't broken by future patches, we need set up mochitest for it.
Depends on: 932151
Assignee: nobody → xyuan
Blocks: 944397
Attached patch add mochitest v1.patch (obsolete) — Splinter Review
Comment on attachment 8344592 [details] [diff] [review]
add mochitest v1.patch

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

Basic tests for mozInputMethod.
Attachment #8344592 - Flags: review?(fabrice)
Comment on attachment 8344592 [details] [diff] [review]
add mochitest v1.patch

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

Thanks for starting that. It's pretty good, but you can simplify somewhat!

::: dom/inputmethod/mochitest/inputmethod_common.js
@@ +26,5 @@
> +    // Backup the old pref values.
> +    prefs.forEach(function(v) {
> +      let name = v[0];
> +      this._boolPrefs.push([name, this._getBoolPref(name)]);
> +    }.bind(this));

You don't need to save old values when using pushPrefEnv

@@ +34,5 @@
> +  addPermissions: function(permissions) {
> +    this._permissions.push(permissions);
> +    permissions.forEach(function(v) {
> +      SpecialPowers.addPermission(v, true, document);
> +    });

Please use pushPermissions() instead of addPermission()

@@ +42,5 @@
> +    this._permissions.forEach(function(v) {
> +      SpecialPowers.removePermission(v, document);
> +    });
> +    SpecialPowers.pushPrefEnv({set: this._boolPrefs}, callback);
> +  },

You don't need this function.
Attachment #8344592 - Flags: review?(fabrice) → feedback+
Attached patch add mochitest v2.patch (obsolete) — Splinter Review
The patch fixed the issue in inputmethod_common.js of comment 4.

When running test on Firefox without gaia helper addon, there is a "browser.shell" undefined error in the console, as browser.shell is
not available. The patch also fixed this problem.
Attachment #8344592 - Attachment is obsolete: true
Attachment #8346055 - Flags: review?(fabrice)
Blocks: 949059
Comment on attachment 8346055 [details] [diff] [review]
add mochitest v2.patch

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

thanks!
Attachment #8346055 - Flags: review?(fabrice) → review+
@Fabrice, thanks for helping with a load of reviews from me :-)
Status: NEW → ASSIGNED
Keywords: checkin-needed
Backed out for B2G mochitest-4 orange. Please verify that this passes on all affected platforms and include the Android fix from comment 9 before re-landing.
https://hg.mozilla.org/integration/b2g-inbound/rev/e3fe8a0517fe

https://tbpl.mozilla.org/php/getParsedLog.php?id=32098407&tree=B2g-Inbound
Attached patch add mochitest v2.patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=22560c43d713

The above try server results show that the composition methods of the input method API are broken. The patch marked the corresponding test as TODO and disable it.
Attachment #8346055 - Attachment is obsolete: true
Attachment #8350002 - Flags: review+
Attached patch add mochitest v3.patch (obsolete) — Splinter Review
Let's wait for the try server result before check-in.

https://tbpl.mozilla.org/?tree=Try&rev=2f60924327c1
Attachment #8350002 - Attachment is obsolete: true
Attachment #8350005 - Flags: review+
The emulator is green now :-)
Keywords: checkin-needed
Backed out for desktop B2G mochitest orange.
https://hg.mozilla.org/integration/b2g-inbound/rev/3ce6af03ee93

https://tbpl.mozilla.org/php/getParsedLog.php?id=32223839&tree=B2g-Inbound

Please give this a full Try run before requesting checkin again.
Same error on desktop Firefox builds as well.
Flags: in-testsuite+
@Ryan, thank you. I should have a full try run.
Attachment #8350151 - Flags: review+
All input method mochitest tests passed finally.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5ba55c9c5ad6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 952741
Blocks: 955965
Blocks: 955969
Blocks: 955970
Blocks: 955973
Blocks: 955974
Blocks: 955975
Blocks: 956688
You need to log in before you can comment on or make changes to this bug.