Closed Bug 829557 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: Plug-ins, defect)

18 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla22
Tracking Status
firefox18 --- affected
firefox19 + wontfix
firefox20 + verified
firefox21 + verified
firefox22 --- verified

People

(Reporter: adrya.stembridge, Assigned: roc)

References

()

Details

(Keywords: regression, testcase)

Attachments

(4 files)

Attached file 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.
Additionally - no errors are produced in Error Console.
Attachment #701024 - Attachment mime type: application/octet-stream → application/x-shockwave-flash
How to use the SWF file you attached?
Please, post more detailed STR.
Flags: needinfo?(adrya.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:email@a.com?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.
Flags: needinfo?(adrya.stembridge)
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?
Flags: needinfo?(adrya.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.
Flags: needinfo?(adrya.stembridge)
Yep, it would be great to have a simple testcase, no PHP server to install or something like that. Thanks.
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
Component: Untriaged → Plug-ins
Product: Firefox → Core
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.
Flags: needinfo?(adrya.stembridge)
I've created a reproduction here.  

http://170.140.138.214/zctests/ZeroClipboard-1.0.7/test_ZC-1.0.7.html
Flags: needinfo?(adrya.stembridge)
I confirm the freeze on FF 19b3 and Nightly 2013-01-23 on Win 7 x64. Setting this to NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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
Summary: Firefox freezes when a flash movie is played → Firefox freezes when content is copied to clipboard with the Zero Clipboard library (based on Flash)


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.
prompts.tab_modal.enabled = false helps
(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.
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?
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.
Attachment #707973 - Flags: review?
Attachment #707973 - Flags: review? → review?(jschoenick)
I'm guessing John is a better reviewer for this.
Enabling this particular code path to safely reenter Gecko fixes this bug and I think it'll fix bug 832963 too.
Attachment #707976 - Flags: review?(jschoenick)
Blocks: 832963
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 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.
(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.
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 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
Attachment #707973 - Flags: review?(jschoenick) → review?(benjamin)
Attachment #707976 - Flags: review?(jschoenick) → review?(benjamin)
Too late to take a fix for FF19, so we'll maintain FF18's state.
(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.
Is there any chance that this will land into FX20?
(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 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 :-(
Attachment #707973 - Flags: review?(benjamin) → review+
Attachment #707976 - Flags: review?(benjamin) → review+
(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?
Paul, could you verify the fix?
Flags: needinfo?(paul.silaghi)
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>
(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 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
Attachment #707973 - Flags: approval-mozilla-beta?
Attachment #707973 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/260236599b04
https://hg.mozilla.org/mozilla-central/rev/bb36734405a2
https://hg.mozilla.org/mozilla-central/rev/fa08283aff8e
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
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).
Attachment #707973 - Flags: approval-mozilla-beta?
Attachment #707973 - Flags: approval-mozilla-beta+
Attachment #707973 - Flags: approval-mozilla-aurora?
Attachment #707973 - Flags: approval-mozilla-aurora+
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?
I forget this so much I made a webapp to keep it straight:
http://whattrainisitnow.com/
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
Flags: needinfo?(paul.silaghi)
[:ted.mielczarek] LOL, awesome, thanks for that. :)

For posterity: yes, the Aurora and Beta versions that I speculated in the previous comment were correct.
(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
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.
Paul thank you for the nightly link. Tested it with Zero Clipboard ver 1.1.7 and it works.
Woohoo, that's great news.  Thanks, guys!
Verified fixed Aurora 21.0a2 (2013-03-07), FF 20b4 Win 7 x64.
Status: RESOLVED → VERIFIED
Keywords: verifyme
QA Contact: paul.silaghi
Is there a date planned for the release of FF20? I am assuming so.
2013-04-01
(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
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.
We also thank you for taking the time to report this.
You need to log in before you can comment on or make changes to this bug.