820677 - vkb doesn't work anymore in b2g

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Jason Smith [:jsmith]]

Build:

Device - Unagi
Type - Beta 12/11/2012
Prefs - Identity prefed on by default

Steps:

1. Go to https://notoriousb2g.personatest.org/signin in the browser
2. Select the text field
3. Start typing

Expected:

As I type, I should see the letters in the text field.

Actual:

I do not see the letters in the text field as I type.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Jet: Could you find someone to look at this?

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: signin to notoriousb2g

This is today's gaia and m-c build.  I'm about to flash my phone to check that.

But this makes it look like a b2g input issue to me, not an identity issue, no?

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Checked on the device and confirmed, persona inputs glow with input focus and seem to accept text from the keyboard, but no text is visible either in the email field or the password field.

Shane, jrgm, do you have any ideas?

Thanks!
j

Comment 4

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Hey, I just flashed my phone and I'm getting the same bad behavior on the password input field in the wifi interface.  (Settings -> WiFi)

I get keyboard feedback (the keys buzz and the keys i'm hitting grow), but the password field doesn't think it's received any input because the OK button is never enabled.

I am using today's m-c and gaia master.

Jason, can you reproduce this?

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Same for the url bar in the browser.  No worky.

Comment 6

[[:fabrice] Fabrice Desré]

All vkb input is broken. sigh...

relevant error in logcat:
[JavaScript Error: "TypeError: inputText is null" {file: "app://keyboard.gaiamobile.org/js/imes/latin/latin.js" line: 381}]

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Sounds like this isn't a layout issue, but rather a gaia keyboard one.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

cc'ing some people that have worked on keyboard recently. Seems like this is a recent regression.

Comment 9

[David Flanagan [:djf]]

That error in latin.js is a symptom of bad input from higher up.  Latin.js has not changed.

Comment 10

[David Flanagan [:djf]]

The error in latin.js (inputText is null) is caused because keyboard.js sends an object with a null value property.

But keyboard.js is getting that object with the null property from the onfocuschange event from navigator.mozKeyboard. 

So the bad object is coming from gecko, I think.

b2g/chrome/content/forms.js creates the value we want with this code:

  let value = element.value || ""

So it should be non-null at that point.

Next, that value gets sent back to chrome with this line:

    sendAsyncMessage("Forms:Input", getJSON(element));

I think that Forms:Input message goes to the mozKeyboard code, but this is where my knowledge of all this gets fuzzy.

Comment 11

[David Flanagan [:djf]]

I think this is a regression caused by:

# HG changeset patch
# User Gene Lian <[e-mail]>
# Date 1355214653 -28800
# Node ID c7db02efa03d33823ef245957ee752eb9d888c62
# Parent  11626685c1331d803e302d5a0701f5d1c102d5f1
Bug 811615 - Miss file name when passing file by Web Activity (part 2, ObjectWrapper.jsm). r=fabrice, a=blocking-basecamp

There's a subtle bug in the way JS falsy values are handled.  The bug was in there before in a slightly different form and wasn't being triggered. But now "" is converting to null.  I'll attach a patch soon.

Comment 12

[David Flanagan [:djf]]

Attachment: test for null correctly so we don't wrap '' as null

How does this look Fabrice?

Should ObjectWrapper.wrap() be doing anything special for functions? Right now I think it treats them as primitives

Comment 13

[David Flanagan [:djf]]

I don't have a tree in a buildable state right now, so I have not actually tested this patch

Comment 14

[[:fabrice] Fabrice Desré]

Attachment: patch

Slightly better patch, removing the now useless "null" branch. That fixes the vkb for me.

Comment 15

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5c0883fb96f

Comment 16

[Gene Lian (I already quit Mozilla)]

Hi guys,

Really sorry and feel guilty about that. It's my bad. :( It didn't properly handle empty string. Honestly, even before my patch, it would potentially fail to do .wrap("") or .wrap(["", ...]) when the "" is not a property value within an object, though. The following values would also have the same risk:

    null
    undefined
    NaN
    empty string ("")
    0
    false

Anyway, everything is solved now. Thanks David and Fabrice! :)

It seems ObjectWrapper.jsm doesn't have a good testing suite so far. I think I can support to make one since it's a hardcode function and needs to be tested all the time. Fire Bug 821159.

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

/dom/base definitively not Gaia :)

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d5c0883fb96f

Comment 19

[David Flanagan [:djf]]

Reopening this because the patch that regressed the keyboard landed on m-a, and this one needs to follow it, or the keyboard will be broken.

I don't know the right way to make that happen, so I'll just try to make some noise here and I'll add whiteboard labels.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 691520 [details] [diff] [review]
patch

We need this to land bug 811615.

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

It's bb+ so you can land it right away with a=blocking-basecamp.

Comment 22

[[:fabrice] Fabrice Desré]

I landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/37fa83bbd0c8

But we need first to get 811615 to land on b2g18 to push this one. Kyle, 811615 this was backed out, can you take a look?

Comment 23

[Alex Keybl [:akeybl]]

Comment on attachment 691520 [details] [diff] [review]
patch

Assuming this has little risk of regression on Aurora 19 desktop/mobile, a=blocking-basecamp

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/14a47035119f