829557 - Firefox freezes when content is copied to clipboard with the Zero Clipboard library (based on Flash)

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Adria Stembridge]

Attachment: ZeroClipboard.swf

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130104151925

Steps to reproduce:

Attempted to copy content to clipboard using the flash swf app ZeroClipboard.   


Actual results:

Clicked the 'copy' (aka play) button, and Firefox became unresponsive for approximately 60 seconds.   


Expected results:

In Firefox 17, clicking the Copy button resulted in my content being copied to clipboard immediately (an email opened with the pasted content in the body).   I've confirmed that this is specific to Firefox build series 18.

Comment 1

[Adria Stembridge]

Additionally - no errors are produced in Error Console.

Comment 2

[Loic]

How to use the SWF file you attached?
Please, post more detailed STR.

Comment 3

[Adria Stembridge]

There is some confusion in the Description.   ZeroClipboard copies some HTML content to clipboard because I am unable to pass the content with "mailto:[e-mail]?body=somecontent".   Workflow is that users click the movie button, which opens a blank email.  They then press Copy-V (or use the menu) to paste the content in the blank email body.

Apologies for the confusion.  

I'll attach some sample code.  The code works fine with Chrome, IE 7/8/9, and Firefox 17 and below.  The delay/freezing problem began immediately after the version 18 auto update.  Checked same script with another machine still on FF 17, no delay/freezing.  I updated that machine to FF 18, and the freezing problem began.

Comment 4

[Adria Stembridge]

Attachment: Sample code of how ZeroClipboard is utilized

Comment 5

[Loic]

How to use this PHP script?
Do you have a simple HTML testcase with the ZeroClipboard library linked (like Jquery e.g.) in the source code?

Comment 6

[Adria Stembridge]

I've not tried linking to ZeroClipboard.swf from a different server.   If you can give me a couple days I'll temporarily recreate my environment on a public server with some changed/deidentified example code.

Comment 7

[Loic]

Yep, it would be great to have a simple testcase, no PHP server to install or something like that. Thanks.

Comment 8

[Manuela Muntean [Away]]

Adria, a testcase would be very useful. Thank you!

Meanwhile, if you wish, you could please try the following :

1) try this with a clean profile: http://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

2) running in Safe mode: http://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode

Comment 9

[Georg Fritzsche [:gfritzsche]]

Adria, is this the solution you are using:
https://github.com/jonrohan/ZeroClipboard

The demo page there is working fine for me:
http://jonrohan.github.com/ZeroClipboard/#demo
... so we'd need a complete test-case that shows the problem.

Comment 10

[Adria Stembridge]

I've created a reproduction here.  

http://170.140.138.214/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html

Comment 11

[Paul Silaghi, QA [:pauly]]

I confirm the freeze on FF 19b3 and Nightly 2013-01-23 on Win 7 x64. Setting this to NEW.

Comment 12

[Loic]

Confirmed too.
Anyway, Adria, ZC 1.0.7 is outdated, the latest version is 1.1.6 which doesn't freeze. Why do not use this one? Because HTML is pasted as markup but not rendered? 

STR:
1) Open testcase with ZC 1.0.7 http://southeastgenetics.org/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html
2) Click on blue button "Copy To Clipboard"

Result: Firefox freezes immediately (still responds but with difficulty)

Regression range:

m-c
good=2012-12-10
bad=2012-12-11
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=725eb8792d27&tochange=4dfe323a663d

I suspect it's another side-effect of:
Robert O'Callahan — Bug 785348. Part 1: Track when we've called into plugin code. While we're in plugin code, never run the refresh driver. r=mats

Comment 13

[Alice0775 White]

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/bb0dbeb26f1d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121209 Firefox/20.0 ID:20121209213450
Bad[*]:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b30a1fff2d07
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121209 Firefox/20.0 ID:20121209213651
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bb0dbeb26f1d&tochange=b30a1fff2d07

In local build
last Good: bb0dbeb26f1d
First bad: 1869f4cbee0b
Triggered by:
1869f4cbee0b	Robert O'Callahan — Bug 785348. Part 1: Track when we've called into plugin code. While we're in plugin code, never run the refresh driver. r=mats

[*] Choose another window, and then reactivate the browser, the freeze is gone.

Comment 14

[Alice0775 White]

prompts.tab_modal.enabled = false helps

Comment 15

[Adria Stembridge]

(In reply to Loic from comment #12)
> Confirmed too.
> Anyway, Adria, ZC 1.0.7 is outdated, the latest version is 1.1.6 which
> doesn't freeze. Why do not use this one? Because HTML is pasted as markup
> but not rendered? 

Correct. That's a separate issue with ZeroClipboard (ticket opened with the owners of that project).  Until ZC fixes that (assuming they do), I'm stuck with 1.0.7.  Our temporary workaround is Chrome, though our users strongly prefer Firefox.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Roc can you take a look at this and let us know what options we may have given that we're two betas away from 19 release - seems like backing out the regressing bug re-introduces a topcrash, but leaving this in -- do we have a way to know how many users might hit this freeze?

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

Benjamin, let me know if you're not the right reviewer for these patches.

These are closely related to bug 785348 which I probably should have had you review.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm guessing John is a better reviewer for this.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Part 2: Allow plugin code to reenter Gecko safely while while the plugin is processing an input event

Enabling this particular code path to safely reenter Gecko fixes this bug and I think it'll fix bug 832963 too.

Comment 20

[Benjamin Smedberg]

Comment on attachment 707973 [details] [diff] [review]
Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

I think I should have understood the original bug better. What does UNSAFE_TO_REENTER_GECKO actually mean? It really is quite important that plugins be allowed to use NPRuntime in some of the calls that are currently marked as UNSAFE.

For example, nsNPAPIPluginInstance::HandleEvent/NPP_HandleEvent very frequently causes plugin to call back into the browser via NPRuntime, and this really must be allowed. And it's fairly common for Flash scripts to do things like call alert() or confirm(), and the Flash player also spins a nested event loop when showing some kinds of context menus. So I don't think we can realistically prevent any useful action from happening in this case, and our own code should be designed to be "safe" in most of these cases.

If we need to delay specific actions (such as NPP_Destroy) to a "safe" point in the event loop, we should do that. We already have code to prevent destroying a plugin instance while that instance is on the stack, and we could extend that code to delay the destruction to more cases.

Comment 21

[Benjamin Smedberg]

Comment on attachment 707976 [details] [diff] [review]
Part 2: Allow plugin code to reenter Gecko safely while while the plugin is processing an input event

So I didn't read the other patch here ;-) But I'd still like to understand why we're defaulting to "unsafe", and what the actual limitations imposed by the unsafe setting are.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #20)
> I think I should have understood the original bug better.

Sorry, I should have had you review the patches in that bug.

> What does
> UNSAFE_TO_REENTER_GECKO actually mean? It really is quite important that
> plugins be allowed to use NPRuntime in some of the calls that are currently
> marked as UNSAFE.

It means that we want to avoid running arbitrary Gecko code under this call site.

We don't *prevent* callbacks to Gecko under such call sites, but when we can "opt out" of some Gecko processing --- in the only case implemented so far, by avoiding running the refresh driver --- we do that.

> For example, nsNPAPIPluginInstance::HandleEvent/NPP_HandleEvent very
> frequently causes plugin to call back into the browser via NPRuntime, and
> this really must be allowed.

The patches here make HandleEvent (for input events) designated safe for reentry, so plugin callbacks to the browser can do everything, as before.

> And it's fairly common for Flash scripts to do
> things like call alert() or confirm(), and the Flash player also spins a
> nested event loop when showing some kinds of context menus. So I don't think
> we can realistically prevent any useful action from happening in this case,
> and our own code should be designed to be "safe" in most of these cases.

That would be ideal but it's hard.

> If we need to delay specific actions (such as NPP_Destroy) to a "safe" point
> in the event loop, we should do that. We already have code to prevent
> destroying a plugin instance while that instance is on the stack, and we
> could extend that code to delay the destruction to more cases.

Apparently this is very hard, hence the fix we made in bug 785348. See comment 7 there.

Comment 23

[Alex Keybl [:akeybl]]

It's unlikely we'll fix for FF19 at this point (this is a non-critical regression in FF18), but we'll definitely take a fix for FF20 once ready.

Comment 24

[John Schoenick [:johns]]

Comment on attachment 707973 [details] [diff] [review]
Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

So "unsafe to re-enter gecko" has an implicit "(but we are anyway)" after it, and is a flag to just avoid any skip-able processing? Right now it sounds a lot like "should not re-enter gecko", but we cannot avoid it in several situations.

Other than the naming of it, this looks okay to me, but I'm not at all familiar with a lot of the HandleEvent codepaths, so I think it would be better to make bsmedberg (or josh?) review this after all

Comment 25

[Alex Keybl [:akeybl]]

Too late to take a fix for FF19, so we'll maintain FF18's state.

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to John Schoenick [:johns] from comment #24)
> Comment on attachment 707973 [details] [diff] [review]
> Part 1: When calling into plugin code, identify situations where it is safe
> (or unsafe) to reenter Gecko from plugin code
> 
> So "unsafe to re-enter gecko" has an implicit "(but we are anyway)" after
> it, and is a flag to just avoid any skip-able processing?

Yes.

Comment 27

[Michał Lipiński]

Is there any chance that this will land into FX20?

Comment 28

[Georg Fritzsche [:gfritzsche]]

(In reply to Michael Lipinski from comment #27)
> Is there any chance that this will land into FX20?

Fx20 just went to beta and the bug is tracked for 20, so it's possible that it will be approved and land there.

Comment 29

[Benjamin Smedberg]

Comment on attachment 707973 [details] [diff] [review]
Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

I dislike this solution, but since it appears the correct solution of delaying plugin stop is too risky until johns fixes bug 767653, I can go with this for now :-(

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f73b7b8d1a2d
https://hg.mozilla.org/integration/mozilla-inbound/rev/622b744e5df9

Comment 31

[Ed Morley [:emorley]]

Push backed out for failing to compile on Windows (so far):
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=dba7a059ed22

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=0a91da5f5eab

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/260236599b04
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb36734405a2

Comment 33

[Daniel Holbert [:dholbert]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #32)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/260236599b04

This cset changed mach to be non-executable:
  https://hg.mozilla.org/integration/mozilla-inbound/diff/260236599b04/mach

Presumably that was accidental?

Comment 34

[Daniel Holbert [:dholbert]]

I reverted that mach executable-bit-removal:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/fa08283aff8e

Comment 35

[Georg Fritzsche [:gfritzsche]]

Paul, could you verify the fix?

Comment 36

[Iraklis Mathiopoulos]

Hello. Confirming that this bug exists in the following setup:

Windows 8 - FF 19.0.1 - ZeroClipboard v1.1.7 - Flash 11.6.602.171

Here is a simple functional test:
----------------------------------

<!doctype html>
<html lang="en">
<head>
	<meta charset="utf-8">
	<title>ZeroClip FF 18 test</title>
	<meta name="description" content="ZeroClip FF 18 test">
	<meta name="author" content="Iraklis Mathiopoulos">
</head>
<body>

	<button id="copy-button" data-clipboard-text="Copy Me!" title="Click to copy me.">Copy to Clipboard</button>
	<script src="ZeroClipboard.js"></script>
	
	<script language="JavaScript">

	var clip = new ZeroClipboard( document.getElementById('copy-button') );

		clip.on( 'complete', function ( client, args ) {
			alert("Copied key " + args.text +" to clipboard");
		} );

	</script>

</body>
</html>

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Daniel Holbert [:dholbert] from comment #34)
> I reverted that mach executable-bit-removal:
>  https://hg.mozilla.org/integration/mozilla-inbound/rev/fa08283aff8e

Thanks. No idea how that happened. I don't even know how to change the executable bit on Windows!

Comment 38

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 707973 [details] [diff] [review]
Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 785348
User impact if declined: some Java applets and Flash objects won't work in certain specialized situations (blank windows, unresponsive dialogs)
Testing completed (on m-c, etc.): Just landed
Risk to taking this patch (and alternatives if risky): This is pretty low risk. We're re-allowing a behavior that we used to allow.
String or UUID changes made by this patch: None

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/260236599b04
https://hg.mozilla.org/mozilla-central/rev/bb36734405a2
https://hg.mozilla.org/mozilla-central/rev/fa08283aff8e

Comment 40

[Alex Keybl [:akeybl]]

Comment on attachment 707973 [details] [diff] [review]
Part 1: When calling into plugin code, identify situations where it is safe (or unsafe) to reenter Gecko from plugin code

Given the low risk evaluation, the fact that this is a recent regression, and the prevalence of ZeroClipboard's usage, we'll try to get a fix in for FF20. If we see any plugin regressions from this though, we'll backout and wontfix for FF20 (maintaining the known bad state from FF18).

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ffaefaf5e1ea
https://hg.mozilla.org/releases/mozilla-aurora/rev/23cb9b70ca1c

Part 1 doesn't apply cleanly to beta, so not pushed there for now.

Comment 42

[James M. Greene]

With regard to :RyanVM's last post:
Can someone please confirm the following, given my lack of knowledge about the current Firefox release cycle?

Current Aurora = FF 21
Current Beta   = FF 20

Correct?

Comment 43

[(not currently active) Ted Mielczarek]

I forget this so much I made a webapp to keep it straight:
http://whattrainisitnow.com/

Comment 44

[Paul Silaghi, QA [:pauly]]

Tested using the STR in comment 12.
The copied result is rendered HTML without any freeze/hang.
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20130305 Firefox/22.0

Comment 45

[James M. Greene]

FYI, the related filed issues for ZeroClipboard are:
 - https://github.com/jonrohan/ZeroClipboard/issues/73
 - https://github.com/jonrohan/ZeroClipboard/issues/85

Comment 46

[James M. Greene]

[:ted.mielczarek] LOL, awesome, thanks for that. :)

For posterity: yes, the Aurora and Beta versions that I speculated in the previous comment were correct.

Comment 47

[Adria Stembridge]

@Paul Silaghi

Which ZeroClipboard version did you test with?

http://170.140.138.214/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html
or
http://170.140.138.214/zctests/ZeroClipboard-1.1.6/test_ZC-1.1.6.html

Thanks.

Comment 48

[Paul Silaghi, QA [:pauly]]

(In reply to Adria Stembridge from comment #47)
> @Paul Silaghi
> 
> Which ZeroClipboard version did you test with?
> 
> http://170.140.138.214/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html
> or
> http://170.140.138.214/zctests/ZeroClipboard-1.1.6/test_ZC-1.1.6.html
> 
> Thanks.
the first one

Comment 49

[Iraklis Mathiopoulos]

To avoid any confusion: 

http://170.140.138.214/zctests/ZeroClipboard-1.1.6/test_ZC-1.1.6.html

does not reproduce the freeze that is experienced in FF18 and FF19. The freeze is caused by calling "alert()" within the "complete" callback of Zero Clipboard, which in this example is commented out.

Comment 50

[Paul Silaghi, QA [:pauly]]

http://170.140.138.214/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html does reproduce it and it's fixed now in Nightly 22.0a1 (2013-03-05) ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-03-05-03-08-33-mozilla-central

Comment 51

[Iraklis Mathiopoulos]

Paul thank you for the nightly link. Tested it with Zero Clipboard ver 1.1.7 and it works.

Comment 52

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-beta/rev/95eaf9a8cb7b
https://hg.mozilla.org/releases/mozilla-beta/rev/2c851ecf2db5

Comment 53

[James M. Greene]

Woohoo, that's great news.  Thanks, guys!

Comment 54

[Paul Silaghi, QA [:pauly]]

Verified fixed Aurora 21.0a2 (2013-03-07), FF 20b4 Win 7 x64.

Comment 55

[James M. Greene]

Is there a date planned for the release of FF20? I am assuming so.

Comment 56

[Paul Silaghi, QA [:pauly]]

2013-04-01

Comment 57

[Georg Fritzsche [:gfritzsche]]

(In reply to James Greene from comment #55)
> Is there a date planned for the release of FF20?

In general, you can find the planned release dates here:
https://wiki.mozilla.org/RapidRelease/Calendar

Comment 58

[Adria Stembridge]

Just wanted to bookend this request by confirming that the issue is resolved in my application (confirmed FF20 on WinXP and FF21 on Win7).

Thanks to the Mozilla team for your hard work.

Comment 59

[Paul Silaghi, QA [:pauly]]

We also thank you for taking the time to report this.