Closed Bug 820677 Opened 10 years ago Closed 10 years ago

vkb doesn't work anymore in b2g

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: jsmith, Assigned: fabrice)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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.
blocking-basecamp: --- → ?
Jet: Could you find someone to look at this?
Assignee: nobody → bugs
blocking-basecamp: ? → +
Attached image 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?
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
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?
Same for the url bar in the browser.  No worky.
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}]
Summary: Signing into the email address field at https://notoriousb2g.personatest.org/signin in the browser shows no text input as I type → vkb doesn't work anymore in b2g
Sounds like this isn't a layout issue, but rather a gaia keyboard one.
Assignee: bugs → nobody
Component: General → Gaia::Keyboard
QA Contact: wachen
cc'ing some people that have worked on keyboard recently. Seems like this is a recent regression.
Assignee: nobody → ttaubert
That error in latin.js is a symptom of bad input from higher up.  Latin.js has not changed.
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.
I think this is a regression caused by:

# HG changeset patch
# User Gene Lian <clian@mozilla.com>
# 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.
How does this look Fabrice?

Should ObjectWrapper.wrap() be doing anything special for functions? Right now I think it treats them as primitives
Attachment #691510 - Flags: review?(fabrice)
I don't have a tree in a buildable state right now, so I have not actually tested this patch
Assignee: ttaubert → dflanagan
Attached patch patchSplinter Review
Slightly better patch, removing the now useless "null" branch. That fixes the vkb for me.
Assignee: dflanagan → fabrice
Attachment #691510 - Attachment is obsolete: true
Attachment #691510 - Flags: review?(fabrice)
Attachment #691520 - Flags: review?(clian)
Attachment #691520 - Flags: review?(clian) → review+
Keywords: regression
Priority: -- → P1
Target Milestone: --- → B2G C3 (12dec-1jan)
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.
/dom/base definitively not Gaia :)
Component: Gaia::Keyboard → DOM: Core & HTML
Product: Boot2Gecko → Core
QA Contact: wachen
https://hg.mozilla.org/mozilla-central/rev/d5c0883fb96f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: approval-mozilla-aurora? approval-mozilla-b2g18?
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Whiteboard: approval-mozilla-aurora? approval-mozilla-b2g18?
Comment on attachment 691520 [details] [diff] [review]
patch

We need this to land bug 811615.
Attachment #691520 - Flags: approval-mozilla-b2g18?
Attachment #691520 - Flags: approval-mozilla-aurora?
It's bb+ so you can land it right away with a=blocking-basecamp.
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 on attachment 691520 [details] [diff] [review]
patch

Assuming this has little risk of regression on Aurora 19 desktop/mobile, a=blocking-basecamp
Attachment #691520 - Flags: approval-mozilla-b2g18?
Attachment #691520 - Flags: approval-mozilla-aurora?
Whiteboard: Needs to be checked in for b2g18 *after* bug 811615.
https://hg.mozilla.org/releases/mozilla-b2g18/rev/14a47035119f
Whiteboard: Needs to be checked in for b2g18 *after* bug 811615.
You need to log in before you can comment on or make changes to this bug.