Closed Bug 688438 Opened 13 years ago Closed 12 years ago

[VKB] Typed characters not displayed in landscape when using VKB

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox9+ fixed, fennec9+)

RESOLVED FIXED
Firefox 15
Tracking Status
firefox9 + fixed
fennec 9+ ---

People

(Reporter: camelia.urian, Assigned: ehsan.akhgari)

References

Details

(Keywords: regression)

Attachments

(1 file)

Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110922 Firefox/9.0a1 Fennec/9.0a1
Device:  HTC Desire Z

Steps to reproduce:
1. Start Fennec in landscape mode.
2. Make sure hard keyboard is closed if available.
3. Tap twice the URL bar to open VKB.
4. Start typing url address.

Actual results:
  Typed characters are not displayed.

Expected results:
  Typed characters are correctly displayed.

Notes:
 - The characters are typed, just not displayed on screen.
 - After releasing the VKB, you can see the typed characters.
Can we get a screenshot? The fullscreen keyboard is not part of Fennec. When the fullscreen keyboard is open, you can't see any part of Fennec. So if the characters are not appearing, I think it's a VKB problem. After you close the VKB, you _do_ see the characters in Fennec.
After closing the VKB, you can see the typed characters. Please see attached video at: http://www.youtube.com/watch?v=lJq_a_mXtVc

Issue is not reproducing on previous nightly build: 
Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110921 Firefox/9.0a1 Fennec/9.0a1
Possible regression from bug 612128. :(
Blocks: 612128
Keywords: regression
So I can try backing that patch out again, but that will leave me with no way to proceed debugging this.  I really want to figure out what's going on here, but I need somebody who can reproduce this and who can also build to try to debug what's going wrong.  :(
I cannot reproduce this on HTC T-Mobile G2 (Android 2.3) using Android Keyboard, Swiftkey X, or Swype.  I'll see if I can reproduce it on any other devices here.
I can reproduce this on my LG Optimus Black.
But I'm afraid I can't build to try to debug.
tracking-fennec: --- → ?
(In reply to Matt Brubeck (:mbrubeck) from comment #5)
> I cannot reproduce this on HTC T-Mobile G2 (Android 2.3) using Android
> Keyboard, Swiftkey X, or Swype.  I'll see if I can reproduce it on any other
> devices here.

Oops, I was testing the wrong build before.  I actually *can* reproduce this on my G2 with the latest nightly build.  Ehsan, I'd be happy to help debug this, and sorry Android is causing so many troubles for your patch.  :(  Can we back it out (again) in the meantime?
The "isReadOnly" variable in "set ActivePanel" is set to true on both my first and second tap because BrowserUI._isKeyboardFullScreen() returns true.  (The other clauses in the isReadOnly assignment statement evaluate to false both times.)
I don't think this is related to the isReadOnly code in AwesomePanel.js, because this bug seems to affect every text field both in chrome and in web content.  I also tried hard-coding "isReadOnly = false" and it did not fix this bug.
Attached patch Patch (v1)Splinter Review
This patch fixes the IME code to also take readwrite elements into account when searching for editable nodes.  This is required because those elements no longer have the NODE_IS_EDITABLE flag set after bug 612128.

Matt confirmed that this fixes the Fennec regression.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #561903 - Flags: review?(bzbarsky)
Try run for f68cb85052e7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f68cb85052e7
Results (out of 171 total builds):
    exception: 1
    success: 146
    warnings: 22
    failure: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-f68cb85052e7
I verified issue with the try-build from:  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-f68cb85052e7
and also with latest nightly: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110926 Firefox/9.0a1 Fennec/9.0a1

Issue is not reproducing anymore.
Boris - Can you suggest another reviewer, if you're swamped? Masayuki?
tracking-fennec: ? → 9+
Comment on attachment 561903 [details] [diff] [review]
Patch (v1)

r=me, though maybe we should have an nsIContent method for this....
Attachment #561903 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky (:bz) from comment #15)
> Comment on attachment 561903 [details] [diff] [review] [diff] [details] [review]
> Patch (v1)
> 
> r=me, though maybe we should have an nsIContent method for this....

That would encourage people to use that, but it's not what we want in almost all cases.  Using that method incorrectly can cause more bugs similar to bug 612128.  So I'd rather not do that.  :-)
Pushed to try to make sure that everything is working correctly before pushing this and bug 612128 to inbound.
Try run for 9bdf60a2a2cb is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=9bdf60a2a2cb
Results (out of 193 total builds):
    success: 150
    warnings: 20
    failure: 23
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-9bdf60a2a2cb
This has been fixed by backout of bug 612128; leaving this bug open so that the real fix can be landed and verified when bug 612128 re-lands.
(In reply to Mozilla RelEng Bot from comment #18)
> Try run for 9bdf60a2a2cb is complete.
> Detailed breakdown of the results available here:
>     https://tbpl.mozilla.org/?tree=Try&rev=9bdf60a2a2cb
> Results (out of 193 total builds):
>     success: 150
>     warnings: 20
>     failure: 23
> Builds available at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.
> com-9bdf60a2a2cb

Masayuki, do you know what needs to be done about these failures?  I'm not sure what the desired behavior should be here...
(In reply to Ehsan Akhgari [:ehsan] from comment #20)
> (In reply to Mozilla RelEng Bot from comment #18)
> > Try run for 9bdf60a2a2cb is complete.
> > Detailed breakdown of the results available here:
> >     https://tbpl.mozilla.org/?tree=Try&rev=9bdf60a2a2cb
> > Results (out of 193 total builds):
> >     success: 150
> >     warnings: 20
> >     failure: 23
> > Builds available at
> > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.
> > com-9bdf60a2a2cb
> 
> Masayuki, do you know what needs to be done about these failures?  I'm not
> sure what the desired behavior should be here...

Don't enable IME on non-editable element. That breaks our IME state management. Your patch makes user be able to use IME on form controls (like checkbox). It provides very bad experience for IME users.

Why do you need to enable IME on such element which isn't text inputtable element?
Note that the "editable" in IME related code means "text editable", doesn't mean "form control state editable".
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #21)
> Don't enable IME on non-editable element. That breaks our IME state
> management. Your patch makes user be able to use IME on form controls (like
> checkbox). It provides very bad experience for IME users.
> 
> Why do you need to enable IME on such element which isn't text inputtable
> element?

I'm not.  Note that with bug 612128, text controls do not have the NODE_IS_EDITABLE flag any more, but they will always have the readwrite state.  This patch is changing the IME subsystem to correctly account for that, and it shouldn't (and doesn't) change any logic of which elements will have the IME state turned on.

I debugged these test failures, and they're the result of yet another bug, bug 694880.  I have a patch on that bug, which fixes these test failures.  :-)
Depends on: 694880
Oh, OK. I see. Thanks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/269ee0275709
Target Milestone: --- → Firefox 10
https://hg.mozilla.org/mozilla-central/rev/269ee0275709
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111024 Firefox/10.0a1 Fennec/10.0a1
Device:  HTC Desire Z
OS: Android 2.3

Type characters are correctly displayed in landscape mode using VKB.
Status: RESOLVED → VERIFIED
Backed out because of bug 688423.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e8312683e7b
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/integration/mozilla-inbound/rev/4651104e0d0e
Target Milestone: Firefox 10 → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/4651104e0d0e
Status: REOPENED → RESOLVED
Closed: 13 years ago12 years ago
Resolution: --- → FIXED
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → +
blocking+, please nom for approval after a few days of baking
Wait, why is this suddenly a blocker?  It doesn't even affect fennec 1.0.  Clearing the flag.
blocking-fennec1.0: + → ---
You need to log in before you can comment on or make changes to this bug.