The default bug view has changed. See this FAQ.

Flash can open VKB even when the plugin element does not have focus

VERIFIED FIXED in Firefox 14

Status

()

Firefox for Android
Keyboards and IME
VERIFIED FIXED
5 years ago
8 months ago

People

(Reporter: cpeterson, Assigned: snorp)

Tracking

({flashplayer})

unspecified
Firefox 15
ARM
Android
flashplayer
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox14 verified, firefox15 fixed, blocking-fennec1.0 betaN+)

Details

(Whiteboard: VKB, URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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?

Updated

5 years ago
blocking-fennec1.0: --- → ?
(Reporter)

Comment 2

5 years ago
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.
blocking-fennec1.0: ? → soft
(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.

Updated

5 years ago
Duplicate of this bug: 750229
Adrian found another test-case with the URL: http://studio1290.com/emphatic-put-down-the-drink-studio1290/
Duplicate of this bug: 750899
It seems that Flash inside iframes is triggering the VKB: http://people.mozilla.org/~mwargers/tests/plugins/flash/iframe_flash.html
(Reporter)

Comment 8

5 years ago
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
Created attachment 626573 [details] [diff] [review]
Don't show vkb when plugin is focused on Android
Attachment #626573 - Flags: review?(cpeterson)
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.
(Reporter)

Comment 11

5 years ago
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.
Attachment #626573 - Flags: review?(cpeterson) → review+
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/00b634ca080f
Filed bug 758226 to address the wonkyness with IME states

Updated

5 years ago
Blocks: 757218
https://hg.mozilla.org/mozilla-central/rev/00b634ca080f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
(Reporter)

Updated

5 years ago
status-firefox14: --- → affected
status-firefox15: --- → fixed
Aurora nom?
Verified fixed on:
Nightly 15.0a1 (2012-05-29) 
Device: Galaxy SII (Android 2.3.4)

Will this be nominated for Aurora?
status-firefox15: fixed → verified
Duplicate of this bug: 757218
Verified Nightly 06/06/2012 on Galaxy S II
Status: RESOLVED → VERIFIED
blocking-fennec1.0: soft → ?
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:
Attachment #626573 - Flags: approval-mozilla-beta?
blocking-fennec1.0: ? → betaN+
Attachment #626573 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/9aa0ab133758
https://hg.mozilla.org/releases/mozilla-beta/rev/de602174c439
status-firefox14: affected → fixed
Unable to reproduce on:
Beta 14.0a7 Build 3
Device: Galaxy SII (Android 2.3.4)
status-firefox15: verified → fixed
Verified for 14 b7 candidate 3 on Samsung Galaxy S II and Samsung Galaxy Nexus
status-firefox14: fixed → verified
You need to log in before you can comment on or make changes to this bug.