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)
Tracking
(firefox9+ fixed, fennec9+)
RESOLVED
FIXED
Firefox 15
People
(Reporter: camelia.urian, Assigned: ehsan.akhgari)
References
Details
(Keywords: regression)
Attachments
(1 file)
2.41 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•13 years ago
|
||
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.
Reporter | ||
Comment 2•13 years ago
|
||
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
Comment 3•13 years ago
|
||
Possible regression from bug 612128. :(
Updated•13 years ago
|
Keywords: regression
Assignee | ||
Comment 4•13 years ago
|
||
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. :(
Comment 5•13 years ago
|
||
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.
Comment 6•13 years ago
|
||
I can reproduce this on my LG Optimus Black. But I'm afraid I can't build to try to debug.
tracking-fennec: --- → ?
Comment 7•13 years ago
|
||
(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?
Comment 8•13 years ago
|
||
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.)
Comment 9•13 years ago
|
||
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.
Assignee | ||
Comment 10•13 years ago
|
||
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.
Comment 11•13 years ago
|
||
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
Reporter | ||
Comment 12•13 years ago
|
||
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.
Comment 13•13 years ago
|
||
Boris - Can you suggest another reviewer, if you're swamped? Masayuki?
tracking-fennec: ? → 9+
tracking-firefox9:
--- → ?
Updated•13 years ago
|
Comment 15•13 years ago
|
||
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+
Assignee | ||
Comment 16•13 years ago
|
||
(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. :-)
Assignee | ||
Comment 17•13 years ago
|
||
Pushed to try to make sure that everything is working correctly before pushing this and bug 612128 to inbound.
Comment 18•13 years ago
|
||
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
Comment 19•13 years ago
|
||
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.
status-firefox9:
--- → fixed
Assignee | ||
Comment 20•13 years ago
|
||
(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...
Comment 21•13 years ago
|
||
(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?
Comment 22•13 years ago
|
||
Note that the "editable" in IME related code means "text editable", doesn't mean "form control state editable".
Assignee | ||
Comment 23•13 years ago
|
||
(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
Comment 24•13 years ago
|
||
Oh, OK. I see. Thanks.
Assignee | ||
Comment 25•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/269ee0275709
Target Milestone: --- → Firefox 10
Comment 26•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/269ee0275709
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 27•13 years ago
|
||
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
Assignee | ||
Comment 28•13 years ago
|
||
Backed out because of bug 688423. https://hg.mozilla.org/integration/mozilla-inbound/rev/3e8312683e7b
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 29•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4651104e0d0e
Target Milestone: Firefox 10 → Firefox 15
Comment 30•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4651104e0d0e
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
blocking-fennec1.0: --- → ?
Updated•12 years ago
|
blocking-fennec1.0: ? → +
Comment 31•12 years ago
|
||
blocking+, please nom for approval after a few days of baking
Assignee | ||
Comment 32•12 years ago
|
||
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.
Description
•