1001325 - [Keyboard] "onselectionchange" event by changing cursor location does not occur

Status: RESOLVED FIXED

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

Description

[Takuya Nakayama]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140423030202

Steps to reproduce:

1. Implement the source codes below.

window.navigator.mozInputMethod.inputcontext.onselectionchange = function() {
  console.log('onselectionchange');
}


2. Input a text string into a text field.
   At this time, input text string is not be selected.

   Example:　test|　　("|" means a cursor location)

3. Move the cursor location from the location Step No.2.
   * You can move the cursor by any operations, such as tapping on the field or tapping Left/Right key.

　 Example:　te|st 　 ("|" means a cursor location)


Actual results:

A log is not outputted.
(A cursor location is changed at Step No.3, however "onselectionchange" event does not occur.)


Expected results:

A log should be outputted.

The text string at Step No.2 and No.3 are same, not changed.
However we believe that "onselectionchange" event should occur 
because "inputcontext" property and "selectionEnd" property of "inputcontext" are changed.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like Gaia doesn't use selection listener... I'm not sure the reason why it's so. But I believe that Gaia should use it or firing events from nsIWidget::NotifyIME().

I know Gonk doesn't implement nsIWidget::NotifiyIME() yet. I guess that it must cause some behaviors are different from other platforms during composition.

Any ideas?

Comment 2

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

If so, it is a bug. Let me confirm it later.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

This works fine on my Peak against the browser URL bar. What could go wrong is that every time you get an inputcontextchange event you'll have to re-attach the onselectionchange handler. So:

navigator.mozInputMethod.addEventListener('inputcontextchange', function() {
  if (!navigator.mozInputMethod.inputcontext) return;
    navigator.mozInputMethod.inputcontext.onselectionchange = function(ev) {
    console.log('onselectionchange', ev.target.selectionStart, ev.target.selectionEnd);
  }
});

Comment 4

[Takuya Nakayama]

Thank you for your reply.

We have modified a source code as you pointed out and checked it with built-in keyboard in local environment.
However logs were still not output. 

We have also tried it with browser text fields other than URL bar, logs are not output.

These are our operating environment and modification detail below.

[Operating Environment]

Device: Peak
OS version: 2.1.0.0-prerelease
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0
Build ID: 20140619132442


[Modification Detail]
File: B2G/gaia/apps/keyboard/js/keyboard.js

Modification Detail:
(We have added source code to '>' part.)

  window.navigator.mozInputMethod.oninputcontextchange = function() {
    perfTimer.printTime('inputcontextchange');
    fakeAppObject.inputContext = navigator.mozInputMethod.inputcontext;
  
    if (fakeAppObject.inputContext) {
>     fakeAppObject.inputContext.onselectionchange = function(ev) {
>       console.log('onselectionchange', ev.target.selectionStart, ev.target.selectionEnd);
>     }
      inputContextGetTextPromise = fakeAppObject.inputContext.getText();
    }
  
    ...
    
  }

Is there anything wrong with our way?

Comment 5

[Jan Jongboom [:janjongboom] (inactive)]

If I paste your code into the keyboard app it works fine...

  window.navigator.mozInputMethod.oninputcontextchange = function() {
    console.log('inputcontextchange');
    var fakeAppObject = {};
    fakeAppObject.inputContext = navigator.mozInputMethod.inputcontext;
  
    if (fakeAppObject.inputContext) {
      fakeAppObject.inputContext.onselectionchange = function(ev) {
        console.log('onselectionchange', ev.target.selectionStart, ev.target.selectionEnd);
      }
    }
  }

Can you share your app with me? You can send it to the email listed here in bugzilla.

Comment 6

[Takuya Nakayama]

Thank you for your reply.

Today, we have got a source code again and tried to do operation check.

These are our operating environment below.

[Operating Environment]
Gaia: bcb2115984ffdabc873f392678723f151cd880fd
Gecko: af6ad08f8657d4f1729595937cca38b8afa65db2

Because we have added modifications to built-in keyboard this time, so we believe there are any special applications we can share. 
we use provided applications.

Please let us know the procedure when logs output.

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Hmm, interesting. I can reproduce this on latest geeksphone build indeed. Thanks for reporting. Let's make a new manual build from mc and see if it's different.

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

tested with bug 1026997.

So the state change populates all the way through MozKeyboard.js, and thats OK.       

      this._fireEvent("selectionchange", {
        selectionStart: ctx.selectionStart,
        selectionEnd: ctx.selectionEnd
      }); 

gets called as well, but the client is never notified nonetheless. No clue whats going on. Unit tests run fine, just a thing on device... Any clue?

Comment 9

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

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #8)
> tested with bug 1026997.
> 
> So the state change populates all the way through MozKeyboard.js, and thats
> OK.       
> 
>       this._fireEvent("selectionchange", {
>         selectionStart: ctx.selectionStart,
>         selectionEnd: ctx.selectionEnd
>       }); 
> 
> gets called as well, but the client is never notified nonetheless. No clue
> whats going on. Unit tests run fine, just a thing on device... Any clue?

I'm not sure what causes this. I can reproduce on flame with current master build.
It seems a DOM binding issue that |this.__DOM_IMPL__.dispatchEvent(event)| fails.

Comment 10

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

Attachment: gaia patch to test onselectionchange

This is the gaia patch I used to test onselection "event".

Comment 11

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

Attachment: Gecko patch to test onselectionchange

This is the gecko patch to log the onselectionchange event.

Comment 12

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

@kanru, we get a strange issue with DOM binging that |this.__DOM_IMPL__.dispatchEvent(event)| cannot dispatch event to content page. The code is:
http://mxr.mozilla.org/mozilla-central/source/dom/inputmethod/MozKeyboard.js#462

But the similar code within the same file works fine:
http://mxr.mozilla.org/mozilla-central/source/dom/inputmethod/MozKeyboard.js#243

Do you know what may cause this problem?

Steps to reproduce this bug:

1. apply the above two patches to master branch, build the repo and flash your device.

2. connect your device to computer and run `adb logcat | grep yxl` to show the log

3. tap the search bar on the homescreen and type anything

Expected:

The log `yxl onselectionchange` goes with each `yxl _fireEvent`

Actual:

Only `yxl _fireEvent` is found, but not ``yxl onselectionchange`

Comment 13

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

I found the cause of this bug.

From MDN[1], I got that "To create a JS-implemented WebIDL object, one must create both the chrome-side implementation object and the content-side page-exposed object."

The WebIDL object - MozInputContext, retured from navigator.mozInputMethod.inputcontext, is not correctly bound to its content-side object. We get invalid this.__DOM_IMPL__ from MozInputContext object, and therefore fail to call |this.__DOM_IMPL__.dispatchEvent(event)|.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Implementing_WebIDL_using_Javascript

Comment 14

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

Attachment: Create valid webidl object.

A content-side object can be created for a given chrome-side object by invoking the static _create method on the interface.

I use the "_create method" to create the WebIDL object for inputcontext from the chrome code.

This way fixes the issue of Comment 13.

The try result:
https://tbpl.mozilla.org/?tree=Try&rev=d42ed8df5789

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/7806e4f88e6f

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7806e4f88e6f

Comment 17

[Makoto Kato [:m_kato]]

Yuan, could you request approval‑mozilla‑b2g32?  3rd party Japanese IME developer needs this fix on 2.0.

Comment 18

[Makoto Kato [:m_kato]]

This is fixed by mozilla33, not mozilla32 and Firefox OS 2.0.

Comment 19

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

Comment on attachment 8450082 [details] [diff] [review]
Create valid webidl object.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1001325
User impact if declined: Event handlers directly attached to navigator.mozInputMethod.inputcontext don't work and maybe 3rd-party keyboard apps rely on it.
Testing completed: yes, https://tbpl.mozilla.org/?tree=Try&rev=d42ed8df5789
Risk to taking this patch (and alternatives if risky): No risk. The patch fixed a WebIDL binding issue for keyboard api and won't impact the built-in keyboard.
String or UUID changes made by this patch: None.

Comment 20

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

It needs checkin on mozilla‑b2g32 for Firefox OS 2.0.
Thanks.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2c922f58c8bf