890207 - [B2G][Gaia][Keyboard][Auto-correction] If a wording include special characters then moving the cursor to the end of a line, the cursor will disappear.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[William Hsu [:whsu]]

* Description:
  This case happened on Gaia master and B2g18.
  if you put a wording between two special characters and move the cursor to the end of the a line then tap space button, the cursor will disappear.
  The following is the demo video. You can download it to get the clear version.
  -> https://dc1.safesync.com/LMmDYKWv/VID_0002.3gp?a=TfEBh-3JjVU

* Reproduction steps:
  1. Launching message app.
  2. Tapping "Pencil" icon to edit a new message
  3. Tapping the message field
  4. Type "@@"
  5. Moving the cursor to the middle of the two special characters.
  6. Typing a wording on it ex: @text@
  7. Moving the cursor to the end of the line
  8. Taping space button

* Expected result:
  The cursor is still there.

* Actual result:
  The cursor disappeared.

* Reproduction build:(Mozilla-b2g18-unagi-eng/2013-07-03-07-02-10)
  + Mercurial-Information
    - Gecko revision="a078412b1671"
    - Gaia revision=""
  + Git-information
    - Gecko revision="38e6b257a832fbd6c91fac54bf8a8c5d3572e3fb"
    - Gaia revision="b12b6f8895427708fad4e25af180266085bd2e74"

Thanks!

Comment 1

[William Hsu [:whsu]]

Other way to reproduce this case.

* Reproduction steps:
  1. Launching message app.
  2. Tapping "Pencil" icon to edit a new message
  3. Tapping the message field
  4. Type "Wee" then tap "To:" field.
  5. Tapping the message field again.
  6. Tap [space] key

* Actual result:
  - The cursor disappear.

Comment 2

[David Flanagan [:djf]]

William,

When the cursor disappears, can the user keep typing? If the user does keep typing, does the cursor come back?  Also, does this only happen in the SMS message field, or can you also reproduce it in other auto-correct text fields (like in UI test)?  Assuming that this is SMS only, I'm asking Yuan Xulei to take a look at this since he knows the contenteditable backend.

I'm nominating this as a koi blocker. But if the user can't keep typing in this case, we should see if we can get a fix into 1.1.

Mihai: are you working on this?  If not, please unassign yourself so someone else can take it.

Comment 3

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Hi David, thanks for reminding me of this one, I kind of abandoned it since it wasn't a blocker.

To answer your questions:

> When the cursor disappears, can the user keep typing? If the user does keep
> typing, does the cursor come back?

Yes, the user can keep on typing and all characters are shown, cursor reappears and works normally.

> Also, does this only happen in the SMS message field, or can you
> also reproduce it in other auto-correct text fields (like in UI test)?

Indeed this was only reproducible in the SMS message field, I couldn't reproduce it in the UI tests > Keyboard > input type=text | textarea fields.

Some more info: this occurs regardless if a string is surrounded by special characters, if you modify the text you already typed and then tap to go to the end of it and tap space, the cursor disappears, e.g.:

Type the following and then tap between words (where ^ points) to add some characters:
"This is test"
        ^
"This is a test"
               ^
Now tapping to go back to the end of the string and tapping space, no cursor is shown.

I'll give it a try to solve it tomorrow, if I can't find the cause of this I will unassign myself.

Comment 4

[William Hsu [:whsu]]

(In reply to David Flanagan [:djf] from comment #2)
> When the cursor disappears, can the user keep typing?
[whsu] Yes, I still can input character into text field

> If the user does keep typing, does the cursor come back?
[whsu] Yes, the cursor will come back if you keep typing.

> Also, does this only happen in the SMS message field,
> or can you also reproduce it in other auto-correct text fields (like in UI test)?
> Assuming that this is SMS only,
[whsu] Now, I only can reproduce this case on Message app. I "cannot" reproduce this case on other apps.

> I'm asking Yuan Xulei to take a look at this since he knows the contenteditable backend.
> I'm nominating this as a koi blocker. But if the user can't keep typing in
> this case, we should see if we can get a fix into 1.1.
[whsu] Thanks for your help! Yes, please do so. ^^

Comment 5

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

I've been trying to go around the SMS contentEditable logic for the past couple of hours but, since I am not that familiar with it, I couldn't find the exact cause of this bug.

Unassigning myself for now so that people who worked on it can have a look.

Comment 6

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

David && Miha:

I can reproduce the bug on phone device, but not b2g-desktop. Since all code regarding b2g keyboard is written by JavaScript, the bug should be reproducible on b2g-desktop if caused by b2g code.

So I guess it is not a b2g specific bug, and may be caused by some C++ code of gecko editor.

Ehsan:
 I found you had fixed caret invisible bug of contenteditable before. Could you help to identify issue?

Comment 7

[(no longer active)]

Is there any test case that shows this problem on b2g-desktop?  Also, is this reproducible in b2g master builds?

It would be really helpful if somebody can extract the DOM structure as it is after pressing Space, and also dump out the window.getSelection().anchorOffset/anchorNode/focusOffset/focusNode attributes.

Comment 8

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

I tested master build on b2g-desktop and can't reproduce this problem. Haven't tested b2g18 build on b2g-desktop. 

But on the phone, I can 100% reproduce this problem in both b2g master and b2g 18 build with the steps in the bug description.

Comment 9

[(no longer active)]

(In reply to comment #8)
> I tested master build on b2g-desktop and can't reproduce this problem. Haven't
> tested b2g18 build on b2g-desktop. 
> 
> But on the phone, I can 100% reproduce this problem in both b2g master and b2g
> 18 build with the steps in the bug description.

Having this happen on b2g but not b2g-desktop is *very* surprising to me...  But FWIW if this is deemed a blocker then I can take a look.

Comment 10

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

Attachment: Fix message editing view

Hi Ehsan, I made a mistake that this happens on b2g-desktop master build as well. 

You may need this path of gaia to test on b2g-desktop.

Comment 11

[Ivan Tsay (:ITsay)]

Likely a cursor management issue

Comment 12

[Preeti Raghunath(:Preeti)]

Yuan,

Any progress on this bug? Its a koi+ and don't see a comment since 8/7

Comment 13

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

No, I was not active for this bug as I know little about the cursor management and didn't have enough resource on it.

Ehsan, do you have time to work on it or shed a light on it?

If nobody want to take this bug, I can have a try, but I really expect an expert to help with this bug.

Comment 14

[Neeraj]

Attachment: Proposed patch for this bug.

Hi,

Here is the proposed patch! Please help me by reviewing this patch(https://github.com/mozilla-b2g/gaia/pull/12856). white-space: pre-wrap seems to solve the issue.

Regards,

Comment 15

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

Comment on attachment 817127 [details] [diff] [review]
Proposed patch for this bug.

Cool!
The patch resolved the issue, but I don't understand why whitespaces collapsing causes it.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

The PR says that the Travis CI run failed on it. Makes me nervous about merging it. Please re-set checkin-needed if I'm worrying in vain :)

Comment 17

[(no longer active)]

(In reply to Yuan Xulei [:yxl] from comment #13)
> Ehsan, do you have time to work on it or shed a light on it?

I don't have a ton of time.  Does the wallpaper fix in gaia unblock us here?  If not, assign this to me, I guess.

Comment 18

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

(In reply to Ryan VanderMeulen [:RyanVM UTC-4][PTO 10/26 - 11/3] from comment #16)
> The PR says that the Travis CI run failed on it. Makes me nervous about
> merging it. Please re-set checkin-needed if I'm worrying in vain :)

I checked with the error and it wasn't caused by this pull request. Other pull requests encountered the same error(for example, https://travis-ci.org/mozilla-b2g/gaia/jobs/12565751).

So it safe to merge.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Thanks for checking :)

Master: https://github.com/mozilla-b2g/gaia/commit/37032f754c7186a9e7e1bf4e848db94ca2584a4c

John, can you please uplift to v1.2? :)

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

[v1.2 922de46]

Comment 21

[William Hsu [:whsu]]

Thanks for all your help!
Verified it.

* Test Build:
 - Gaia:     606517ceafe0950c2b89822d5f13353743334f2c
 - Gecko:    http://hg.mozilla.org/releases/mozilla-aurora/rev/5eabd267ef04
 - BuildID   20131025004000
 - Version   26.0a2

Comment 22

[Ed Morley [:emorley]]

Making this disappear from the saved search for uplifts.