Closed Bug 1360666 Opened 7 years ago Closed 6 years ago

BigBlueButton Flash does not receive click events after switching tabs when 32-bit async drawing is enabled

Categories

(Core Graveyard :: Plug-ins, defect, P3)

54 Branch
x86
Windows
defect

Tracking

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 disabled, firefox55+ wontfix, firefox56+ fix-optional, firefox57+ fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- disabled
firefox55 + wontfix
firefox56 + fix-optional
firefox57 + fixed

People

(Reporter: ffdixon, Assigned: jimm)

References

Details

(Keywords: flashplayer, regression)

Attachments

(3 files, 8 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce:

We created a short video to demonstrate the bug

  https://youtu.be/UAt7wHeksG0

Here are the steps to reproduce:

1. Go to https://demo.bigbluebutton.org/
2. Enter "FireFox" for meeting Name
3. Click 'Start' Button
4. Enter Name (such as "John Do")
5. Click 'Join' button

BigBlueButton will load (BigBlueButton is an open source web conferencing system for online learning).

6.  Click 'Cancel' in the audio dialog
7.  Click on the chat text field and type "Hi"
8.  Click 'Send' button

Mouse events are picked up fine.

9.  Open a new tab in the browser
10. Click back to the BigBlueButton Tab



Actual results:

After step 10, user is unable to click anywhere within the BigBlueButton Flash application.  It appears to no longer receive mouse down events (though keyboard events are getting through.


Expected results:

After step 10, user should have been able to click into the BigBlueButton application and, for example, send another chat message.

This worked fine in all previous versions of FireFox 53 (and earlier)

The bug also appears when the FireFox window is minimized and maximized.
Component: Untriaged → Plug-ins
Keywords: flashplayer
Product: Firefox → Core
Stefan can you do some testing around this on the normal axes: win32/win64, async drawing, etc.
Flags: needinfo?(stefan.georgiev)
Keywords: regression
I have tested the reported issue on the latest Nightly build  and latest Firefox release and I manage to reproduce it only on 32 bits version on both channels. 

When flipping dom.ipc.plugins.asyncdrawing.enabled; to false the issue is not reproducible.

The issue is not reproducible on any 64bits version.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(stefan.georgiev)
Hardware: Unspecified → x86
Too late to fix in 53 but we could still take a patch for 54. 
Benjamin, can you help find an owner for this bug? Is it something you want to see fixed for 55/54?
Flags: needinfo?(benjamin)
Not me -> jimm
Flags: needinfo?(benjamin) → needinfo?(jmathies)
Flags: needinfo?(jmathies)
> status-firefox54: affected → unaffected

Jim marked 54 as unaffected because we are disabling Flash async drawing (for 32-bit Firefox) in Beta 54 (bug 1363611).
Depends on: 1363611
OS: Unspecified → Windows
Yay! 

I just tried FireFox 54.0b7 (32-bit) on Windows 10 and now the BigBlueButton (Flash) client is receiving mouse clicks again.  Thanks guys!

Regards,... Fred
I just tested this in Nightly 64-bit, flash release 25.0.0.171 and I can't reproduce.

Tracy could you please test in 32 and 64 bit 55 nightly with async drawing turned on to see if you can repo?
Flags: needinfo?(twalker)
Tested 32-bit nightly, no issue with those builds either.
Beta 54 is affected again now that we re-enabled Flash async drawing for 64-bit Firefox in bug 1363876.
64-bits Firefox is not affected per comment #2.
Following original STR's on Win 10 Surface Pro all with dom.ipc.plugins.asyncdrawing.enabled = true:

64-bit Nightly 55.0a1  (20170524030204) - WFM
32-bit Nightly 55.0a1  (20170524030204) - WFM
64-bit Firefox 54.0b10 (20170522172523) - WFM
32-bit Firefox 54.0b10 (20170522172523) - WFM
Flags: needinfo?(twalker)
Given Tracy's test results in comment 11, I think we can resolve this bug as WORKSFORME.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
This issue was fixed in 54, but it appeared again in 55.0b4 (32-bit) and 55.0b5 (32-bit).
xaerok,  did you see it in 55.0b(1,2 or 3? Also, are you using the latest version of Flash, 26.0.0.131 (https://get.adobe.com/flashplayer/)?
Hi Tracy,

I didn't test it in 55.0b(1,2 or 3), I'm not sure how to roll back the updates. Suddenly found it in b4 yesterday, then updated to b5 and still was able to reproduce the bug there.

Confirming - I'm using Flash 26.0.0.131, last updated on June 17.
As the original poster of the issue, I can also confirm it has reappeared in 55.0b5 (32-bit) under Flash 26.0.0.131.

Regards,... Fred
Hi, 

I see this bug is marked as RESOLVED WORKSFORME, but it reappeared in FireFox 55-beta.  

Regards,... Fred
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Summary: Flash does not receive click events after switching tabs in FireFox 54-beta → Flash does not receive click events after switching tabs in FireFox 55-beta
[Tracking Requested - why for this release]:

I can reproduce this bug in 32-bit Beta 55, but not 64-bit. Flipping the pref dom.ipc.plugins.asyncdrawing.enabled = false fixes the problem.
Summary: Flash does not receive click events after switching tabs in FireFox 55-beta → BigBlueButton Flash does not receive click events after switching tabs when 32-bit async drawing is enabled
Flags: needinfo?(jmathies)
Seems like a recent regression. Tracked for 55.
Hi there,

We're still seeing the issue in 55.0b8 (32-bit).  

If this bug issue gets into release it would be a big problem for our customers as they would be unable to use BigBlueButton after switching tabs in FireFox.

This occurred in FireFox 54 and was fixed.  Can we expect the same outcome for FireFox 55?
Jim, can someone on your team take a look at this Flash async drawing bug? This bug only affects 32-bit Firefox!
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
The demo site seems to redirect to a 404 not found page. If I visit 

https://demo.bigbluebutton.org/

I get redirected to 

https://demo.bigbluebutton.org/b

which is a 404.
Flags: needinfo?(jmathies) → needinfo?(ffdixon)
Flags: needinfo?(jmathies)
Hi Jim,

It's back up!  (You caught us in the middle of an update).  Try it again and let us know if it works now.

Regards,.. Fred
Flags: needinfo?(ffdixon)
This is simplified on nightly now that we prompt for flash activation. The prompt plays the same role as flipping tabs in chrome. A simple work around is to click on the address bar and then back in flash.

I wouldn't consider this blocking but I will try to find a fix before we ship.
bug 1311975 is a possible dupe.
Blocks: 1311975
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
Hi Jim,

Thanks for the update.  

BigBlueButton runs in a tab in FireFox.  We have a feature for breakout rooms that launches the user into a new tab.  After students have finished the breakout room, they return to the first tab.

Our worry is that if users get stuck when the return the first tab (i.e. they need to click somewhere that is not obvious before the UI responds), we'll get a lot of support requests.

We really need it to not block keyboard inputs when switching between tabs.  We really want this to work like it does in the current release (FF 54).

Regards,.. Fred
(In reply to ffdixon from comment #26)
> Hi Jim,
> 
> Thanks for the update.  
> 
> BigBlueButton runs in a tab in FireFox.  We have a feature for breakout
> rooms that launches the user into a new tab.  After students have finished
> the breakout room, they return to the first tab.
> 
> Our worry is that if users get stuck when the return the first tab (i.e.
> they need to click somewhere that is not obvious before the UI responds),
> we'll get a lot of support requests.
> 
> We really need it to not block keyboard inputs when switching between tabs. 
> We really want this to work like it does in the current release (FF 54).
> 
> Regards,.. Fred

Unrelated but I'm curious, do you have an initiative to move away from flash eventually?
Hi Jim,

We are building in parallel an HTML5 client that can run alongside BigBlueButton.  See

  https://bigbluebutton.org/2017/05/17/html5-client-dev-release/

We have a lot of users on the Flash client.  Our plan is to increasingly make the HTML5 client feature compatible with the Flash client and then, at some point, switch over.

We figure that process will take us a year or two, and in the meantime we're still working **** improving the Flash client.  We'll be releasing shared notes, multi-user whiteboard, and WebRTC-based desktop sharing for the Flash client this fall.

Thanks again for all your help (and for all the help from Mozilla).  A few years ago we graduated from Mozilla's WebFWD program

  https://webfwd.org/portfolio/

and have been building out BigBlueButton for online education ever since.

Warm Regards,.. Fred
This is somewhere down in KeyboardLayout, TextDispatcher or apz event handling code. Occasionally we get an empty mPluginEvent object way over in content's nsPluginInstanceOwner event handling. I've traced it back to win32 widget code. Will dig more tomorrow.
Jim, you mention in comment 7 that a workaround for this bug is to click focus to the browser address bar and then back to Flash. Is that focus change workaround something that the web page could do with JavaScript, in case we can't fix this bug in time for Firefox 55?

If the page listened for the HTML `visibilitychange` event to detect a tab switch and then programmatically set the input focus to a (hidden?) HTML input form and then back to the Flash element, would that cause Flash to start drawing correctly again?
Hey Masayuki, this appears to be a bug in the input context code. In nsWindow's ExternalHandlerProcessMessage we check PluginHasFocus() before delivering a plugin event. With the str in this bug that call returns false for all keyboard events. You can fix the bug by clicking in the address bar, then clicking in the flash text input.

Curious if you or someone on your team who is familiar with the IME code that detects the presence of plugins can take a look?
Flags: needinfo?(masayuki)
Hmm, sounds like that IMEStateManager or nsFocusManager has a bug. When tab is changed back, IMEStateManager::OnChangeFocus() should get aContent as a plugin owner element, but perhaps, it doesn't receive it.  Then, it'll set input context as ENABLED or DISABLED instead of PLUGIN. Then, PluginHasFocus() returns false.

I think that I need to check it because nobody is aware around here except me :-(

However, I have no idea why dom.ipc.plugins.asyncdrawing.enabled value causes changing the focus behavior. Who is familiar with the design of async drawing?
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #32)
> Hmm, sounds like that IMEStateManager or nsFocusManager has a bug. When tab
> is changed back, IMEStateManager::OnChangeFocus() should get aContent as a
> plugin owner element, but perhaps, it doesn't receive it.  Then, it'll set
> input context as ENABLED or DISABLED instead of PLUGIN. Then,
> PluginHasFocus() returns false.

You can ignore the tab switching part of the str since the current flash activation prompt triggers the same bug. Basically:

1. Go to https://demo.bigbluebutton.org/
2. Enter "FireFox" for meeting Name
3. Click 'Start' Button
4. Enter Name (such as "John Do")
5. Click 'Join' button
6.  Click 'Cancel' in the audio dialog
7.  Click on the chat text field

result: blue highlight without blinking cursor. if you see a blinking cursor the bug won't repo.

8) type in flash text field

result: no text input

9) click on the address bar
10) click on the flash text field again

result: text input works

> I think that I need to check it because nobody is aware around here except
> me :-(

Thanks! I'll continue to look at this from my end as well.

> However, I have no idea why dom.ipc.plugins.asyncdrawing.enabled value
> causes changing the focus behavior. Who is familiar with the design of async
> drawing?

BigBlueButton originally used wmode=window so they would get a child window in content. Keyboard input was routed directly to that window by the system, hence no bug.
Another way to address this is to simply alt-tab away from and back to the browser window. This triggers a call to nsWindow::DispatchFocusToTopLevelWindow which somehow sets up the IME state set properly.
(In reply to Jim Mathies [:jimm] from comment #33)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #32)
> > However, I have no idea why dom.ipc.plugins.asyncdrawing.enabled value
> > causes changing the focus behavior. Who is familiar with the design of async
> > drawing?
> 
> BigBlueButton originally used wmode=window so they would get a child window
> in content. Keyboard input was routed directly to that window by the system,
> hence no bug.

Ah, so, if dom.ipc.plugins.asyncdrawing.enabled is true, does Flash Player always work in windowless mode even on x86 build?

Indeed, I don't see the plugin window even on x86 build on BigBlueButton. On the other hand, I cannot reproduce this bug on my machine. I always see blinking cursor in the textbox even with new STR in comment 33 nor using Alt-Tab. After I go back to the tab for BigBlueButton, I see focus in plugin has gone from the textbox (not having blue outline), then, when I click the textbox to input text, both outline and caret are shown correctly and I can type text. Do I misunderstand something to reproduce this?
On the other hand, on macOS, focus is back to the textbox automatically when tab is switched back or focus is back from other window. I don't know why Flash Player loses focus on Windows, though. Flash Player on Chrome also doesn't lose focus.
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #35)
> Ah, so, if dom.ipc.plugins.asyncdrawing.enabled is true, does Flash Player
> always work in windowless mode even on x86 build?

Yes, in Fx 55 and higher.
Flags: needinfo?(jmathies)
How does the ime code in the parent update it's state to IMEState::PLUGIN? I can't ind that code anywhere. I see a method HTMLObjectElement::GetDesiredIMEState() but that would only get called in the child.
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #35)
> I cannot reproduce this bug on my machine. I always see
> blinking cursor in the textbox even with new STR in comment 33 nor using
> Alt-Tab. After I go back to the tab for BigBlueButton, I see focus in plugin
> has gone from the textbox (not having blue outline), then, when I click the
> textbox to input text, both outline and caret are shown correctly and I can
> type text. Do I misunderstand something to reproduce this?

Will Flash protected mode affect the result?
IMEStateManager in content process gets PLUGIN when focused element's an object element or something plugin owner element and it returns PLUGIN from GetDesiredIMEState().  Then, it sends to PuppetWidget::SetInputContext().  Then, TabParent::RecvSetInputContext() will receive it and it calls IMEStateManager::SetInputContextForChildProcess() in the parent process. Finally, it calls nsIWidget::SetInputContext() of nsWindow.
(In reply to Masatoshi Kimura [:emk] from comment #39)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #35)
> > I cannot reproduce this bug on my machine. I always see
> > blinking cursor in the textbox even with new STR in comment 33 nor using
> > Alt-Tab. After I go back to the tab for BigBlueButton, I see focus in plugin
> > has gone from the textbox (not having blue outline), then, when I click the
> > textbox to input text, both outline and caret are shown correctly and I can
> > type text. Do I misunderstand something to reproduce this?
> 
> Will Flash protected mode affect the result?

Hmm, not related. However, I reproduced only on VM. I guess some Japanese IMEs might affect this since they are using a11y to retrieve host application's content.
When I reproduce this on VM, I got this log:

> [Main Thread]: I/IMEStateManager OnChangeFocus(aPresContext=0x13827800, aContent=0x00000000, aCause=CAUSE_UNKNOWN)
> [Main Thread]: I/IMEStateManager OnChangeFocusInternal(aPresContext=0x13827800 (available: true), aContent=0x00000000 (TabParent=0x00000000), aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x13827800 (available: true), sContent=0x143E5C10, sWidget=0x134C3C00 (available: true), sActiveTabParent=0x00000000, sActiveIMEContentObserver=0x1A2E9900, sInstalledMenuKeyboardListener=false
> [Main Thread]: I/IMEStateManager NotifyIME(aNotification={ mMessage=REQUEST_TO_COMMIT_COMPOSITION }, aWidget=0x134C3C00, aTabParent=0x00000000), sFocusedIMEWidget=0x134C3C00, sActiveTabParent=0x00000000, sFocusedIMETabParent=0x00000000, IsSameProcess(aTabParent, sActiveTabParent)=true, IsSameProcess(aTabParent, sFocusedIMETabParent)=true
> [Main Thread]: I/IMEStateManager   NotifyIME(), the request to IME is ignored because there have been no compositions yet
> [Main Thread]: I/IMEStateManager DestroyIMEContentObserver(), sActiveIMEContentObserver=0x1A2E9900
> [Main Thread]: D/IMEStateManager   DestroyIMEContentObserver(), destroying the active IMEContentObserver...
> [Main Thread]: I/IMEStateManager NotifyIME(aNotification={ mMessage=NOTIFY_IME_OF_BLUR }, aWidget=0x134C3C00, aTabParent=0x00000000), sFocusedIMEWidget=0x134C3C00, sActiveTabParent=0x00000000, sFocusedIMETabParent=0x00000000, IsSameProcess(aTabParent, sActiveTabParent)=true, IsSameProcess(aTabParent, sFocusedIMETabParent)=true
> [Main Thread]: I/IMEStateManager GetNewIMEState(aPresContext=0x13827800, aContent=0x00000000), sInstalledMenuKeyboardListener=false
> [Main Thread]: D/IMEStateManager   GetNewIMEState() returns DISABLED because no content has focus
> [Main Thread]: I/IMEStateManager SetIMEState(aState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, aContent=0x00000000 (TabParent=0x00000000), aWidget=0x134C3C00, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=LOST_FOCUS }, aOrigin=ORIGIN_MAIN)
> [Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x134C3C00, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN_CHROME, mAction=LOST_FOCUS }), sActiveTabParent=0x00000000

Click on the tab, focus is normally moved to no element and IME is disabled properly.

> [Main Thread]: I/IMEStateManager OnChangeFocus(aPresContext=0x13827800, aContent=0x00000000, aCause=CAUSE_UNKNOWN)
> [Main Thread]: I/IMEStateManager OnChangeFocusInternal(aPresContext=0x13827800 (available: true), aContent=0x00000000 (TabParent=0x00000000), aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x13827800 (available: true), sContent=0x00000000, sWidget=0x134C3C00 (available: true), sActiveTabParent=0x00000000, sActiveIMEContentObserver=0x00000000, sInstalledMenuKeyboardListener=false
> [Main Thread]: I/IMEStateManager GetNewIMEState(aPresContext=0x13827800, aContent=0x00000000), sInstalledMenuKeyboardListener=false
> [Main Thread]: D/IMEStateManager   GetNewIMEState() returns DISABLED because no content has focus
> [Main Thread]: D/IMEStateManager   OnChangeFocusInternal(), neither focus nor IME state is changing

Then, I guess that this is called due to moving focus from tab to tab's content, but redundant call and IMEStateManager does nothing.

> [Main Thread]: I/IMEStateManager OnChangeFocus(aPresContext=0x13827800, aContent=0x1A514BA0, aCause=CAUSE_UNKNOWN)
> [Main Thread]: I/IMEStateManager OnChangeFocusInternal(aPresContext=0x13827800 (available: true), aContent=0x1A514BA0 (TabParent=0x1E648C00), aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x13827800 (available: true), sContent=0x00000000, sWidget=0x134C3C00 (available: true), sActiveTabParent=0x00000000, sActiveIMEContentObserver=0x00000000, sInstalledMenuKeyboardListener=false
> [Main Thread]: I/IMEStateManager NotifyIME(aNotification={ mMessage=REQUEST_TO_COMMIT_COMPOSITION }, aWidget=0x134C3C00, aTabParent=0x00000000), sFocusedIMEWidget=0x00000000, sActiveTabParent=0x00000000, sFocusedIMETabParent=0x00000000, IsSameProcess(aTabParent, sActiveTabParent)=true, IsSameProcess(aTabParent, sFocusedIMETabParent)=true
> [Main Thread]: I/IMEStateManager   NotifyIME(), the request to IME is ignored because there have been no compositions yet

Next, the plugin owner element gets focus.

> [Main Thread]: D/IMEStateManager   OnChangeFocusInternal(), will disable IME until new focused element (or document) in the child process will get focus actually
> [Main Thread]: I/IMEStateManager SetIMEState(aState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, aContent=0x1A514BA0 (TabParent=0x1E648C00), aWidget=0x134C3C00, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=GOT_FOCUS }, aOrigin=ORIGIN_CONTENT)
> [Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x134C3C00, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN_CHROME, mAction=GOT_FOCUS }), sActiveTabParent=0x00000000

IME is disabled temporarily until SetInputContext() is called by the remote process.

> [Main Thread]: I/IMEStateManager SetInputContextForChildProcess(aTabParent=0x1E648C00, aInputContext={ mIMEState={ mEnabled=PLUGIN, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sPresContext=0x13827800 (available: true), sWidget=0x134C3C00 (available: true), sActiveTabParent=0x1E648C00

Then, it comes as expected.

> [Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x134C3C00, aInputContext={ mIMEState={ mEnabled=PLUGIN, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sActiveTabParent=0x1E648C00
> [Main Thread]: I/IMEStateManager NotifyIME(aNotification={ mMessage=NOTIFY_IME_OF_FOCUS }, aWidget=0x134C3C00, aTabParent=0x1E648C00), sFocusedIMEWidget=0x00000000, sActiveTabParent=0x1E648C00, sFocusedIMETabParent=0x00000000, IsSameProcess(aTabParent, sActiveTabParent)=true, IsSameProcess(aTabParent, sFocusedIMETabParent)=false

Finally, IMEStateManager sets InputContext to PLUGIN and notifies NOTIFY_IME_OF_FOCUS as expected. So, I don't know why even with this, cannot input text.

On the other hand, I cannot recover with Jim's workaround. So, I might reproduce different symptom. I cannot click any button in the Flush Player anymore.
Attached file ime log.txt
here's an ime manager log of the failure.
Assignee: nobody → jmathies
This is the last bit of logging that happens right after enabling flash and the plugin starts to load - 

[Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x000000000AA4DC00, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sActiveTabParent=0x0000000000000000
mozilla::widget::PuppetWidget::SetInputContext cause=0 focus=1 enabled=0
mozilla::dom::TabParent::RecvSetInputContext aIMEEnabled=0
[Main Thread]: I/IMEStateManager SetInputContextForChildProcess(aTabParent=0x0000000014C08800, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sPresContext=0x0000000017DC6000 (available: true), sWidget=0x0000000016B20000 (available: true), sActiveTabParent=0x0000000014C08800
[Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x0000000016B20000, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sActiveTabParent=0x0000000014C08800

doesn't look like content ime mamager picks up that a plugin has loaded. don't know why yet.
So I think I've found the issue, not sure how to fix it though - right after the user clicks the flash activate button, the Type of the HTMLObjectElement is still at eType_Loading.

------ calling into GetNewIMEState in child..
[Main Thread]: I/IMEStateManager GetNewIMEState(aPresContext=0x00000000072CE800, aContent=0x000000000EE07400), sInstalledMenuKeyboardListener=false
mozilla::dom::HTMLObjectElement::GetDesiredIMEState Type=0
[Main Thread]: D/IMEStateManager   GetNewIMEState() returns { mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }
------ done
More complete set of logging features that show the problem:

------ calling into GetNewIMEState in child..
[Main Thread]: I/IMEStateManager GetNewIMEState(aPresContext=0x0000000005EDF000, aContent=0x00000000078E2A00), sInstalledMenuKeyboardListener=false
*** content queries HTMLObjectElement for ime state:
mozilla::dom::HTMLObjectElement::GetDesiredIMEState Type=0
[Main Thread]: I/IMEStateManager OnChangeFocusInternal(aPresContext=0x0000000017FA5000 (available: true), aContent=0x0000000014828280 (TabParent=0x000000000D761800), aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x0000000017FA5000 (available: true), sContent=0x0000000000000000, sWidget=0x0000000016C81800 (available: true), sActiveTabParent=0x0000000000000000, sActiveIMEContentObserver=0x0000000000000000, sInstalledMenuKeyboardListener=false
[Main Thread]: I/IMEStateManager NotifyIME(aNotification={ mMessage=REQUEST_TO_COMMIT_COMPOSITION }, aWidget=0x0000000016C81800, aTabParent=0x0000000000000000), sFocusedIMEWidget=0x0000000000000000, sActiveTabParent=0x0000000000000000, sFocusedIMETabParent=0x0000000000000000, IsSameProcess(aTabParent, sActiveTabParent)=true, IsSameProcess(aTabParent, sFocusedIMETabParent)=true
[Main Thread]: I/IMEStateManager   NotifyIME(), the request to IME is ignored because there have been no compositions yet
[Main Thread]: D/IMEStateManager   OnChangeFocusInternal(), will disable IME until new focused element (or document) in the child process will get focus actually
[Main Thread]: I/IMEStateManager SetIMEState(aState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, aContent=0x0000000014828280 (TabParent=0x000000000D761800), aWidget=0x0000000016C81800, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=GOT_FOCUS }, aOrigin=ORIGIN_CONTENT)
[Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x0000000016C81800, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN_CHROME, mAction=GOT_FOCUS }), sActiveTabParent=0x0000000000000000
[Main Thread]: D/IMEStateManager   GetNewIMEState() returns { mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }
------ done
[Main Thread]: I/IMEStateManager SetIMEState(aState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, aContent=0x00000000078E2A00 (TabParent=0x0000000000000000), aWidget=0x0000000003308C00, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=GOT_FOCUS }, aOrigin=ORIGIN_CONTENT)
[Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x0000000003308C00, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sActiveTabParent=0x0000000000000000
mozilla::widget::PuppetWidget::SetInputContext cause=0 focus=1 enabled=0
mozilla::dom::TabParent::RecvSetInputContext aIMEEnabled=0
[Main Thread]: I/IMEStateManager SetInputContextForChildProcess(aTabParent=0x000000000D761800, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sPresContext=0x0000000017FA5000 (available: true), sWidget=0x0000000016C81800 (available: true), sActiveTabParent=0x000000000D761800
[Main Thread]: I/IMEStateManager SetInputContext(aWidget=0x0000000016C81800, aInputContext={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN, mAction=GOT_FOCUS }), sActiveTabParent=0x000000000D761800
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Channel OnStartRequest
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: LoadObject called, notify 1, forceload 0, channel 0000000005E49070
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Updating object parameters
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Channel has a content type of application/x-shockwave-flash
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Using plugin type hint in favor of any channel type
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Using channel type
*** loading -> plugin:
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Type changed from 0 -> 2
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: LoadObject - plugin state changed (2)
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Notifying about state change: (0, 20000) -> (2, 0) (sync 1, notify 1)
[Main Thread]: D/objlc OBJLC [00000000078E2AA0]: Notifying about state change: (2, 0) -> (2, 0) (sync 0, notify 1)
[Main Thread]: D/objlc OBJLC [00000000078E2A00]: nsSimplePluginEvent firing event "PluginInstantiated"
Spawned Process Attacher: Attached to process, PID=13092
This fixes the keyboard events issue. There's another problem where the plugin occasionally misses a WM_SETFOCUS event. Flash clearly depends on this for getting it's focus state right. I'll look for a way to trigger sending this in a different asea of the code base.
Attachment #8891472 - Flags: review?(masayuki)
Flags: needinfo?(masayuki)
- cleaner version
Attachment #8891472 - Attachment is obsolete: true
Attachment #8891472 - Flags: review?(masayuki)
Attachment #8891522 - Flags: review?(masayuki)
Comment on attachment 8891522 [details] [diff] [review]
plugin handling in GetDesiredIMEState v.1

Review of attachment 8891522 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/html/HTMLObjectElement.cpp
@@ +409,5 @@
> +    nsCOMPtr<nsIContent> thisContent =
> +      do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
> +    if (thisContent &&
> +        (thisContent->IsHTMLElement(nsGkAtoms::object) ||
> +         thisContent->IsHTMLElement(nsGkAtoms::applet))) {

actually this is a bad check since I'm doing this within an HTMLObjectElement tag. This could still be an image or maybe svg. So I need a better check.
Attachment #8891522 - Flags: review?(masayuki)
(In reply to Jim Mathies [:jimm] from comment #49)
> ::: dom/html/HTMLObjectElement.cpp
> > +    if (thisContent &&
> > +        (thisContent->IsHTMLElement(nsGkAtoms::object) ||
> > +         thisContent->IsHTMLElement(nsGkAtoms::applet))) {

btw, you can omit applet checks because <applet> support is currently being removed in bug 1279218.
Hey Masayuki, is there a method in IMEStateManager that nsObjectLoadingContent could call to trigger a refresh of ime state?
Flags: needinfo?(masayuki)
Thank you for your investigation!

(In reply to Jim Mathies [:jimm] from comment #51)
> Hey Masayuki, is there a method in IMEStateManager that
> nsObjectLoadingContent could call to trigger a refresh of ime state?

There is, but you need to change a bit.  You should be able to use IMEStateManager::UpdateIMEState().  Then, make aEditorBase from EditorBase& to EditorBase* and add nsPresShell* aPresShell.  When EditorBase is not nullptr, it should work as current implementation.  Otherwise, aPresShell is used for |nsCOMPtr<nsIPresShell> presShell|. Then, you don't need to mind the call of  IMEContentObserver::MaybeReinitialize() below because sActiveIMEContentObserver is nullptr in such case (since not ENABLED nor PLUGIN), and similar to the call of CreateIMEContentObserver() because it allows nullptr for the argument when focused content sets InputContext to PLUGIN.
Flags: needinfo?(masayuki)
Attached patch wip (obsolete) — Splinter Review
Attachment #8891522 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
Added a focus check to this which seemed appropriate.
Attachment #8892216 - Attachment is obsolete: true
Attachment #8892539 - Flags: review?(masayuki)
Comment on attachment 8892539 [details] [diff] [review]
patch

>diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp
>@@ -275,16 +275,23 @@ GetContentWindow(nsIContent* aContent)
>     nsIDocument* subdoc = doc->GetSubDocumentFor(aContent);
>     if (subdoc)
>       return subdoc->GetWindow();
>   }
> 
>   return nullptr;
> }
> 
>+bool nsFocusManager::IsFocused(nsIContent* aContent)
>+{
>+  if (!aContent || !mFocusedContent)
>+    return false;

nit: Please put | {| to the end of the previous line's tail. And please insert a line whose text is |  }| to the next line.

>+  return SameCOMIdentity(aContent, mFocusedContent);

I don't understand why you use SameCOMIdentify here. It's expensive and mFocusedContent is nsCOMPtr<nsIContent>. So, |aContent == mFocusedContent| or |aContent == mFocusedContent.get()| should work.

>diff --git a/dom/base/nsObjectLoadingContent.cpp b/dom/base/nsObjectLoadingContent.cpp
>--- a/dom/base/nsObjectLoadingContent.cpp
>+++ b/dom/base/nsObjectLoadingContent.cpp
>@@ -87,17 +87,20 @@
> #include "mozilla/AsyncEventDispatcher.h"
> #include "mozilla/EventDispatcher.h"
> #include "mozilla/EventStateManager.h"
> #include "mozilla/EventStates.h"
> #include "mozilla/IntegerPrintfMacros.h"
> #include "mozilla/dom/HTMLObjectElementBinding.h"
> #include "mozilla/dom/HTMLSharedObjectElement.h"
> #include "mozilla/dom/HTMLObjectElement.h"
>+#include "mozilla/widget/IMEData.h"
> #include "nsChannelClassifier.h"
>+#include "IMEStateManager.h"
>+#include "nsFocusManager.h"

nit: Looks like that the include order is a-z. So, IMEStateManager.h is inserted to wrong place.

>@@ -2001,17 +2004,30 @@ nsObjectLoadingContent::UpdateObjectPara
> 
>   ///
>   /// Update changed values
>   ///
> 
>   if (newType != mType) {
>     retval = (ParameterUpdateFlags)(retval | eParamStateChanged);
>     LOG(("OBJLC [%p]: Type changed from %u -> %u", this, mType, newType));
>+    bool updateIMEState = (mType == eType_Loading && newType == eType_Plugin);
>     mType = newType;
>+    // The IME manager needs to know if this is a plugin so it can adjust input handling

nit: too long line, wrap it after "input".

>+    // to an appropriate mode for plugins.
>+    nsFocusManager* fm = nsFocusManager::GetFocusManager();
>+    nsCOMPtr<nsIContent> thisContent =
>+      do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
>+    NS_ASSERTION(thisContent, "must be a content");

MOZ_ASSERT?
Attachment #8892539 - Flags: review?(masayuki) → review+
Attached patch patch v.2 (obsolete) — Splinter Review
Attachment #8892539 - Attachment is obsolete: true
Attachment #8892905 - Flags: review+
Comment on attachment 8892905 [details] [diff] [review]
patch v.2

oops, old patch.
Attachment #8892905 - Attachment is obsolete: true
Attached patch patch v.2 (obsolete) — Splinter Review
Attachment #8892909 - Flags: review+
Keywords: checkin-needed
Needs a rebased patch.
Flags: needinfo?(jmathies)
Keywords: checkin-needed
Attached patch patch v.3 (obsolete) — Splinter Review
- updated to latest mc
Attachment #8892909 - Attachment is obsolete: true
Flags: needinfo?(jmathies)
Attachment #8893077 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcd223309de8
Update IME state when nsObjectLoadingContent content changes type from 'loading' to a valid content type. r=masayuki
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5697ac761739361957cf58ed9ade77d957cc2384 for causing assertions like  Assertion failure: aContent (we must have content), at /home/worker/workspace/build/src/dom/events/IMEStateManager.cpp:893 in stylo reftests

https://treeherder.mozilla.org/logviewer.html#?job_id=120553188&repo=mozilla-inbound
Flags: needinfo?(jmathies)
Ah, aContent can be nullptr if it's designMode editor. So, the method should take nsPresShell* too.
I don't see how EditorBase can be null except from the call I'm adding from nsObjetcLoadingContent - 

http://searchfox.org/mozilla-central/search?q=UpdateIMEState&path=

-  nsCOMPtr<nsIPresShell> presShell = aEditorBase.GetPresShell();
+  MOZ_ASSERT(aContent, "we must have content");
+
+  nsCOMPtr<nsIPresShell> presShell;
+  if (!aEditorBase) {
+    nsIDocument* doc = aContent->OwnerDoc();
+    presShell = doc->GetShell();
+  } else {
+    presShell = aEditorBase->GetPresShell();
+  }


> Ah, aContent can be nullptr if it's designMode editor. So, the method should take nsPresShell* too.

If aContent is null, then it's a call from EditorBase where we have a pres shell from aEditorBase->GetPresShell().

It looks like I can just remove the content assertion?
Flags: needinfo?(jmathies) → needinfo?(masayuki)
Ah, yes. So, the MOZ_ASSERT can be |aContent || aEditorBase| if you'd like to keep it.
Flags: needinfo?(masayuki)
Looks good, but some nits:

> --- a/dom/base/nsFocusManager.cpp
> +++ b/dom/base/nsFocusManager.cpp
> @@ -275,16 +275,24 @@ GetContentWindow(nsIContent* aContent)
>      nsIDocument* subdoc = doc->GetSubDocumentFor(aContent);
>      if (subdoc)
>        return subdoc->GetWindow();
>    }
>  
>    return nullptr;
>  }
>  
> +bool nsFocusManager::IsFocused(nsIContent* aContent)
> +{

Wrap the line after |bool|.

> +++ b/dom/base/nsObjectLoadingContent.cpp
> @@ -82,21 +82,24 @@
>  #include "mozilla/dom/Element.h"
>  #include "mozilla/dom/Event.h"
>  #include "mozilla/dom/ScriptSettings.h"
>  #include "mozilla/dom/PluginCrashedEvent.h"
>  #include "mozilla/AsyncEventDispatcher.h"
>  #include "mozilla/EventDispatcher.h"
>  #include "mozilla/EventStateManager.h"
>  #include "mozilla/EventStates.h"
> +#include "IMEStateManager.h"
> +#include "mozilla/widget/IMEData.h"
>  #include "mozilla/IntegerPrintfMacros.h"
>  #include "mozilla/dom/HTMLObjectElementBinding.h"
>  #include "mozilla/dom/HTMLEmbedElement.h"
>  #include "mozilla/dom/HTMLObjectElement.h"
>  #include "nsChannelClassifier.h"
> +#include "nsFocusManager.h"

IMEStateManager.h should be mozilla/IMEStateManager.h. Although, sorting only with file name is odd.

With these changes, r=masayuki.
Attached patch patch v.4 (obsolete) — Splinter Review
Attachment #8893077 - Attachment is obsolete: true
Attachment #8893518 - Flags: review+
Attached patch patch v.4Splinter Review
Attachment #8893518 - Attachment is obsolete: true
Attachment #8893519 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd2fbaa64c40
Update IME state when nsObjectLoadingContent content changes type from 'loading' to a valid content type. r=masayuki
Keywords: checkin-needed
ffdixon, this 32-bit Flash bug will be fixed in Firefox 56 (release date 2017-09-26). Unfortunately, it is too late to fix in Firefox 55, which is being released this week.

Until Firefox 56 is released, you might be able to work around this 32-bit Flash bug with some sort of JavaScript hack to reset the input focus to the embedded Flash content when the user switches tabs. I don't know if that would actually work. It's just a guess based on the information in comment 7 and comment 30.
Ugh.  

Our breakout rooms in BigBlueButton depend on the user switching back and forth between tabs, see https://www.youtube.com/watch?v=q5N-lcocJss.  September is the busiest time for us as schools get ready for the fall. 

According to our usage stats, we had 1.3m users in the past 24 months for one of our larger customers alone.  That works out to about 54k users/month.  If just 2% used Breakout Rooms and half of them are using FireFox, that's 540 users which will likely be unable to participate in the class and not understand why.  That's going to be a lot of support tickets just for that customer alone.

It was working fine in FireFox 54 (and earlier).

Is there any way we can get this into FireFox 55 (or FireFox 55.0.1)?  The one saving grace is that August tends to be a slow month, but September is going to be full on for.

Regards,... Fred
Comment on attachment 8893519 [details] [diff] [review]
patch v.4

Approval Request Comment
[Feature/Bug causing the regression]:
e10s regression
[User impact if declined]:
buggy flash focus
[Is this code covered by automated tests?]:
no
[Has the fix been verified in Nightly?]:
yes
[Needs manual test from QE? If yes, steps to reproduce]: 
yes, see bug comments
[List of other uplifts needed for the feature/fix]:
none
[Is the change risky?]:
no
[Why is the change risky/not risky?]:
mostly isolated to plugin code. understood issue.
[String changes made/needed]:
no
Attachment #8893519 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/bd2fbaa64c40
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Let's verify the fix in 57 before we uplift (maybe aiming at 56 beta 2). 
Andre, can your team help? Thanks.
Flags: needinfo?(andrei.vaida)
Attached image 57.0a1.gif
Hi Jim!

I have managed to reproduce this issue by following the steps mentioned in comment 0 using Firefox 54.0b3 (Build Id:20170427091925 , 32bit build) on Windows 10 64bit.

It seems that this issue is not fixed since it is still reproducible on 57.0a1 (Build Id:20170806100257, 32bit build).

Please see the attached screencast for further details.

Could you please take a look into this? 

Thanks!
Flags: needinfo?(andrei.vaida) → needinfo?(jmathies)
We came up with a JavaScript work-around (Thanks Chris for https://bugzilla.mozilla.org/show_bug.cgi?id=1360666#c73).

We added the following JavaScript to the top of our main page BigBlueButton.html to trigger a resize of the applet on FF 55/56 32-bit Win by changing the height of the content by 1% and then back.

    <script type="text/javascript">
      // Workaround Flash hang in Firefox 55 / 56
      // See https://bugzilla.mozilla.org/show_bug.cgi?id=1360666

      // if it's Windows
      if (navigator.userAgent.indexOf("Windows") != -1
        // if it's Firefox 55 or 56
        && (navigator.userAgent.indexOf("Firefox/55") != -1 || navigator.userAgent.indexOf("Firefox/56") != -1)
        // if there is no 64-bit signature
        && (navigator.userAgent.indexOf("Win64; x64") == -1)) {

          // resizing the content block on visibility change, to make Flash responsive again
          document.addEventListener('visibilitychange', function() {
            if(document.visibilityState == 'visible') {
              document.getElementById("content").style.height = "99%";
              setTimeout(function(){ document.getElementById("content").style.height = "100%"; }, 500);
            }
          });
      }
    </script>

To access BigBlueButton without the JavaScript fix, follow the steps in the first comment and, after the page loads, change "BigBlueButton.html" to "BigBlueButton-FF.html".  You'll see in the source it doesn't have the above JavaScript and BigBlueButton does not get mouse clicks/keyboard input when the user tabs away and comes back.

Regards,... Fred
Status: RESOLVED → REOPENED
Flags: needinfo?(jmathies)
Resolution: FIXED → ---
Attachment #8893519 - Flags: approval-mozilla-beta?
We are only 1 week from RC. Mark 56 fix-optional but still happy to have the fix in 56.
We'd be happy to see it land in 56 as well!

Regards,... Fred
BigBlueButton Product Manager
Priority: -- → P3
See Also: → 1405346
Thank you ffdixon for the workaround. 

We have extended the workaround a bit since it also occurred when a user opens the flash page in a maximized browser. We also trigger the resizing when the page finished loading in the affected browsers.

So here is our version:

        // if it's Windows
      	if (navigator.userAgent.indexOf("Windows") != -1
        	// if it's Firefox 55 or 56
        	&& (navigator.userAgent.indexOf("Firefox/55") != -1 || navigator.userAgent.indexOf("Firefox/56") != -1)
        	// if there is no 64-bit signature
        	&& (navigator.userAgent.indexOf("Win64; x64") == -1)) {

          	// resizing the content block on visibility change, to make Flash responsive again
          	window.addEventListener('visibilitychange', function() {
            	    if(document.visibilityState == 'visible') {
              		rescaleFlashApp();
            	    }
          	});
			
	        window.addEventListener('load', function() {
              	    rescaleFlashApp();
          	});
			
			
      	}
		
        function rescaleFlashApp(){
	    document.getElementById("content").style.height = "99%";
            setTimeout(function(){ document.getElementById("content").style.height = "100%"; }, 500);
	}
Has anyone tested this in the latest flash 27 beta? 

http://labs.adobe.com/technologies/flashruntimes/flashplayer/

There's a fix in this release for a related bug.
Just tried it again using Flash 29.0.0.171 running within FireFox 60.0.1 (64-bit) on Windows 10.  No problems at all with switching back and forth.

Good work guys!
(In reply to ffdixon from comment #84)
> Just tried it again using Flash 29.0.0.171 running within FireFox 60.0.1
> (64-bit) on Windows 10.  No problems at all with switching back and forth.

Thanks for testing! I'll close this bug as fixed now.
Status: REOPENED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: