941329 - [mozInputMethod] trailing spaces in contenteditable elements stripped from textBeforeCursor

Status: RESOLVED INCOMPLETE

(Core :: DOM: Device Interfaces, defect)

Description

[David Flanagan [:djf]]

I'm trying to use mozInputMethod.inputcontext.textBeforeCursor in the keyboard app.

When I test with the SMS app, if I use the contenteditable input field and type spaces, textBeforeCursor is always the empty string.

This surprises me because of the fix in bug 902847.

Surprisingly, the bug does not appear when I use UITetsts->Keyboard and try the contenteditable areas in that app.  So there is something about the SMS app that is breaking textBeforeCursor, apparently.

Comment 1

[David Flanagan [:djf]]

Yuan and Jan: do either of you have any ideas about this?

Comment 2

[David Flanagan [:djf]]

Actually, I can't figure out how textBeforeCursor or any other inputcontext state changes after calling sendKey. Unlike replaceSurroundingText(), the async event returned after calling sendKey does not include updated state. It doesn't look like getSelectionInfo() gets called in forms.js at all so I've got no clue what is going on.

Comment 3

[David Flanagan [:djf]]

Changing my keyboard to use replaceSurroundingText instead of sendKey does not fix the problem.

Modifying the SMS app to remove teh <br> element from the contenteditable element does not fix the problem (but I notice that with my new keyboard the br is not necessary and it is necessary with the old keyboard. That's strange.)

On a probably unrelated note, if I type really fast in my new keyboard, I sometimes see a series of these messages in the console: 

E/GeckoConsole(  178): [JavaScript Error: "too much recursion"]

I don't know if it is coming form my code or from the gecko keyboard code.

Comment 4

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

I think we have several tasks to do before completely dropping MozKeyboard API.
Let's solve them one by one.

Comment 5

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

(In reply to David Flanagan [:djf] from comment #2)
> Actually, I can't figure out how textBeforeCursor or any other inputcontext
> state changes after calling sendKey. Unlike replaceSurroundingText(), the
> async event returned after calling sendKey does not include updated state.
> It doesn't look like getSelectionInfo() gets called in forms.js at all so
> I've got no clue what is going on.

It is caused by Bug 860546.

Comment 6

[David Flanagan [:djf]]

Yuan,

I don't understand how this is related to bug 860546. Are you suggesting that the SMS app is stripping off the trailing spaces?  The contenteditable element shows the spaces, but they are not in textBeforeCursor.

This problem only seems to affect my new refactored version of the keyboard. The old keyboard (which does not use textBeforeCursor) does not seem affected. The existing keyboard maintains its own model of the input field state. The new keyboard does too, but after it calls sendKey() it waits for the returned promise to resolve and then verifies that its internal state matches the new state of the input context. This is where I'm having trouble. When I type spaces at the end of the SMS app's contenteditable field, I add a space to my internal copy of textBeforeCursor.  But then when the promise returns I check inputcontext.textBeforeCursor and find that it does not have the space appended to it.  So I adjust my internal copy of the state to match what inputcontext is reporting.  But now the keyboard state is out of sync with what I see on the screen, where the spaces are clearly inserted.

(If you're curious about how what I'm doing, see test_apps/demo-keyboard/js/input_field.js.  I create an InputField module as a wrapper around navigator.mozInputMethod.inputcontext  and use it to avoid asynchronous waits to get the current input state)

What do you mean by comment #4? Are you suggesting that this is a low-priority bug?

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

This is definitely a bug in platform. 

Situation: contenteditable (in SMS) with content: |     Test| (visible to the user).

Put cursor at position 0. In forms.js > getContentEditableText we do:

  let range = doc.createRange();
  range.selectNodeContents(element);

When you execute this you get the following back: { offsetStart: 0, offsetEnd: 6 } which is incorrect. Also when we then do:

  let encoder = FormAssistant.documentEncoder;
  encoder.setRange(range);
  return encoder.encodeToString();

We get |"Test\n"| or when you put cursor right before cursor it's encodeToString |" Test\n"|

So this makes no sense. The thing is that I'm not familiar at all with the internals of contentEditable. :p

Comment 8

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

(In reply to David Flanagan [:djf] from comment #6)
> Yuan,
> 
> I don't understand how this is related to bug 860546. Are you suggesting
> that the SMS app is stripping off the trailing spaces?  The contenteditable
> element shows the spaces, but they are not in textBeforeCursor.

I'm sorry. I should make it clear.
bug 860546 cause "calling sendKey does not include updated state", but is not 
related to the "stripping off the trailing spaces".

> 
> This problem only seems to affect my new refactored version of the keyboard.
> The old keyboard (which does not use textBeforeCursor) does not seem
> affected. The existing keyboard maintains its own model of the input field
> state. The new keyboard does too, but after it calls sendKey() it waits for
> the returned promise to resolve and then verifies that its internal state
> matches the new state of the input context. This is where I'm having
> trouble. When I type spaces at the end of the SMS app's contenteditable
> field, I add a space to my internal copy of textBeforeCursor.  But then when
> the promise returns I check inputcontext.textBeforeCursor and find that it
> does not have the space appended to it.  So I adjust my internal copy of the
> state to match what inputcontext is reporting.  But now the keyboard state
> is out of sync with what I see on the screen, where the spaces are clearly
> inserted.
> 
> (If you're curious about how what I'm doing, see
> test_apps/demo-keyboard/js/input_field.js.  I create an InputField module as
> a wrapper around navigator.mozInputMethod.inputcontext  and use it to avoid
> asynchronous waits to get the current input state)
> 
> What do you mean by comment #4? Are you suggesting that this is a
> low-priority bug?
Not low-priority, but not easy to solve and we may need more than one patch to 
solve it.

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Cleaning up Device Interfaces component, and mass-marking old FxOS bugs as incomplete.

If any of these bugs are still valid, please let me know.