STR: 1. (After bug 707353's showKeyboard patch lands), load http://www.zasmakujegancji.pl/ AR: The VKB will open immediately. ER: The stock browser does not open the VKB until after the user taps input focus to Flash text box.

When tapping on http://people.mozilla.org/~mwargers/tests/plugins/flash/317375_flash_wmode.swf , I get to see the vkb too in current trunk build. That's not what the stock browser is doing. Is that also covered by this bug?

mw22, is that Flash SWF file _supposed_ to open the VKB? If not, that sounds like a new bug (likely a regression from bug 707353). Maybe Flash SWF files loaded directly with a .swf URL (instead of an <embed> tag) automatically have input focus? I wonder if that is the problem with zasmakujegancji.pl, too.

(In reply to Chris Peterson (:cpeterson) from comment #2) > mw22, is that Flash SWF file _supposed_ to open the VKB? If not, that sounds > like a new bug (likely a regression from bug 707353). No, it isn't supposed to, I filed now bug 749120 for it.

Adrian found another test-case with the URL: http://studio1290.com/emphatic-put-down-the-drink-studio1290/

It seems that Flash inside iframes is triggering the VKB: http://people.mozilla.org/~mwargers/tests/plugins/flash/iframe_flash.html

I don't know if this is a different bug, but I see a VKB regression when viewing zombo.com. With Nightly 2012-05-03, there is no VKB (correct behavior), but with Nightly 2012-05-04, the VKB opens automatically (fail). Here is there changeset regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2db9df42823d&tochange=9ebf3dc839c5

A little more explanation is needed. When a plugin is focused, nsIMEStateManager sends calls nsWindow::SetInputContext with IMEState::PLUGIN for mEnabled and IMEState::OPEN_STATE_NOT_SUPPORTED for mOpen. We only pass the value of mEnabled up to GeckoInputConnection. If it is anything other than DISABLED, we show the vkb. That's why it shows up whenever you focus a plugin (which occurs when you tap it or enable it via tap-to-click). In ANPSystem, Flash will explicit tells us whether or not it wants to have the vkb open or closed. To open it, we use IMEState::PLUGIN and IMEState::OPEN. This hack makes it so we only show the vkb when that condition occurs.

Comment on attachment 626573 [details] [diff] [review] Don't show vkb when plugin is focused on Android Review of attachment 626573 [details] [diff] [review]: ----------------------------------------------------------------- LGTM with nits. ::: widget/android/nsWindow.cpp @@ +2086,5 @@ > > + int enabled = int(aContext.mIMEState.mEnabled); > + > + // Only show the virtual keyboard for plugins if mOpen is set appropriately. > + // This avoids showing it whenver a plugin is focused. Bug 747492 s/whenver/whenever/ :) @@ +2088,5 @@ > + > + // Only show the virtual keyboard for plugins if mOpen is set appropriately. > + // This avoids showing it whenver a plugin is focused. Bug 747492 > + if (aContext.mIMEState.mEnabled == IMEState::PLUGIN && > + aContext.mIMEState.mOpen != IMEState::OPEN) { Since the mEnabled/mOpen state combinations are so confusing, maybe we should add some asserts to make sure our assumptions don't change. If IMEState::PLUGIN, maybe something like: MOZ_ASSERT(mOpen == IMEState::OPEN || mOpen == IMEState::OPEN_STATE_NOT_SUPPORTED); Adding asserts for the non-plugin states would be helpful, but are not within the scope of this fix.

(In reply to Chris Peterson (:cpeterson) from comment #11) > Comment on attachment 626573 [details] [diff] [review] > Don't show vkb when plugin is focused on Android > > Review of attachment 626573 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM with nits. > > ::: widget/android/nsWindow.cpp > @@ +2086,5 @@ > > > > + int enabled = int(aContext.mIMEState.mEnabled); > > + > > + // Only show the virtual keyboard for plugins if mOpen is set appropriately. > > + // This avoids showing it whenver a plugin is focused. Bug 747492 > > s/whenver/whenever/ :) Thanks, fixed. > > @@ +2088,5 @@ > > + > > + // Only show the virtual keyboard for plugins if mOpen is set appropriately. > > + // This avoids showing it whenver a plugin is focused. Bug 747492 > > + if (aContext.mIMEState.mEnabled == IMEState::PLUGIN && > > + aContext.mIMEState.mOpen != IMEState::OPEN) { > > Since the mEnabled/mOpen state combinations are so confusing, maybe we > should add some asserts to make sure our assumptions don't change. If > IMEState::PLUGIN, maybe something like: > > MOZ_ASSERT(mOpen == IMEState::OPEN || mOpen == > IMEState::OPEN_STATE_NOT_SUPPORTED); > > Adding asserts for the non-plugin states would be helpful, but are not > within the scope of this fix. Well mOpened can be OPEN, CLOSED, (in response to Flash requests) or OPEN_STATE_NOT_SUPPORTED (focused) when mEnabled is PLUGIN. So I don't know that the assert for PLUGIN really buys us a lot. Adding one for the other states makes more sense to me.

Upon testing some more, we also get DISABLED and OPEN together (!). So I think I'm going to hold off on adding any assertions because it will just look crazy. We do need to get this sorted out in a better way, though.

Filed bug 758226 to address the wonkyness with IME states

Aurora nom?

Verified fixed on: Nightly 15.0a1 (2012-05-29) Device: Galaxy SII (Android 2.3.4) Will this be nominated for Aurora?

Verified Nightly 06/06/2012 on Galaxy S II

Comment on attachment 626573 [details] [diff] [review] Don't show vkb when plugin is focused on Android [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed (on m-c, etc.): Risk to taking this patch (and alternatives if risky): String or UUID changes made by this patch:

https://hg.mozilla.org/releases/mozilla-beta/rev/9aa0ab133758 https://hg.mozilla.org/releases/mozilla-beta/rev/de602174c439

Unable to reproduce on: Beta 14.0a7 Build 3 Device: Galaxy SII (Android 2.3.4)

Verified for 14 b7 candidate 3 on Samsung Galaxy S II and Samsung Galaxy Nexus