Closed Bug 932151 Opened 6 years ago Closed 6 years ago

Enable Keyboard/IME API to run in Nightly without installing helper extension

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: xyuan, Assigned: xyuan)

References

Details

Attachments

(2 files, 2 obsolete files)

Currently we need a gaia extension keyboard@gaiamobile.org(https://github.com/mozilla-b2g/gaia/tree/7d7864ce51f3faac0f8f55e487a10ba5b58d226c/tools/extensions/keyboard) to support Keyboard/IME API in Firefox Nightly. 

Maintaining the extension makes us some trouble:

1. A source code file named forms.js is copied from gecko to that extension. It requires extra effort to sync the source code between gecko and gaia. We got some bugs when not syncing the file in time.

2. We cannot write mochitest for firefox nightly as the extension locates in gaia, not gecko.

So I suggest we remove the gaia extension and support all its functions in gecko.
QA Contact: xyuan
Assignee: nobody → xyuan
QA Contact: xyuan
Attached patch gecko patch v1 (obsolete) — Splinter Review
The patch made the b2g specific file - forms.js available for desktop build. The file won't be loaded until keyboard/IME api is enabled.
Attachment #827949 - Flags: review?(fabrice)
Attached file gaia pull request
Attachment #827952 - Flags: review?(fabrice)
Comment on attachment 827949 [details] [diff] [review]
gecko patch v1

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

looks ok, but I don't see the moved form.js file in this patch.

::: dom/browser-element/BrowserElementChild.js
@@ +24,5 @@
>  
>  
>  if (!('BrowserElementIsPreloaded' in this)) {
> +  try {
> +    if(Services.prefs.getBoolPref("dom.mozInputMethod.enabled")) {

nit: space after if

::: dom/ipc/preload.js
@@ +87,5 @@
>    } catch(e) {
>    }
>  
> +  try {
> +    if(Services.prefs.getBoolPref("dom.mozInputMethod.enabled")) {

nit: if (...)
Attachment #827949 - Flags: review?(fabrice) → feedback+
(In reply to Fabrice Desré [:fabrice] from comment #4)
> Comment on attachment 827949 [details] [diff] [review]
> gecko patch v1
> 
> Review of attachment 827949 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> looks ok, but I don't see the moved form.js file in this patch.
> 
There is a block of moving forms.js to dom/inputmethod/ in the patch
file as following, which doesn't show in the |diff| and |review| pages.
 
  rename from b2g/chrome/content/forms.js
  rename to dom/inputmethod/forms.js
Attached patch gecko patch v2 (obsolete) — Splinter Review
Fix nits.

https://tbpl.mozilla.org/?tree=Try&rev=d46eaa0b46c6
Attachment #827949 - Attachment is obsolete: true
Comment on attachment 828422 [details] [diff] [review]
gecko patch v2

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

The failure of chrome mochitest on Linux Opt of the try seems unrelated.
Attachment #828422 - Flags: review?(fabrice)
Attachment #828422 - Flags: review?(fabrice) → review+
Attachment #827952 - Flags: review?(fabrice) → review+
@Fabrice, thank you:-)
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/f2ed6b24269a

Looks like this needs an updated Gaia PR.
Keywords: checkin-needed
Whiteboard: [leave open]
Whiteboard: [leave open]
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #9)
> https://hg.mozilla.org/integration/b2g-inbound/rev/f2ed6b24269a
> 
> Looks like this needs an updated Gaia PR.

Thank you Ryan. PR updated.
Depends on: 936324
Backed out by timdream's request, for causing bug 936324:
remote:   https://hg.mozilla.org/mozilla-central/rev/f003c386c77a
Attached patch gecko patch v3Splinter Review
Fix a copy-paste error in dom/browser-element/BrowserElementChild.js caused loading chrome://global/content/forms.js failed.

The diff with previous path is:

diff --git a/dom/browser-element/BrowserElementChild.js b/dom/browser-element/BrowserElementChild.js
--- a/dom/browser-element/BrowserElementChild.js
+++ b/dom/browser-element/BrowserElementChild.js
@@ -21,17 +21,17 @@ let infos = sendSyncMessage('browser-ele
                             { 'msg_name': 'hello' })[0];
 docShell.QueryInterface(Ci.nsIDocShellTreeItem).name = infos.name;
 docShell.setFullscreenAllowed(infos.fullscreenAllowed);
 
 
 if (!('BrowserElementIsPreloaded' in this)) {
   try {
     if (Services.prefs.getBoolPref("dom.mozInputMethod.enabled")) {
-      Services.scriptloader.loadSubScript("chrome://global/content/forms.js", global);
+      Services.scriptloader.loadSubScript("chrome://global/content/forms.js");
     }
   } catch (e) {
   }
   // Those are produc-specific files that's sometimes unavailable.
   try {
     Services.scriptloader.loadSubScript("chrome://browser/content/ErrorPage.js");
   } catch (e) {
   }
Attachment #828422 - Attachment is obsolete: true
Attachment #829208 - Flags: review+
Tested on device, The patch doesn't cause bug 936324 any more.
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4eea9a74a8bb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 948462
You need to log in before you can comment on or make changes to this bug.