Closed
Bug 545734
Opened 15 years ago
Closed 15 years ago
need to hide the iframe used for submitting plugin crash reports
Categories
(Toolkit :: Crash Reporting, defect)
Toolkit
Crash Reporting
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a2
Tracking | Status | |
---|---|---|
status1.9.2 | --- | .4-fixed |
People
(Reporter: Dolske, Assigned: Dolske)
References
Details
(Keywords: dogfood, Whiteboard: [fixed-lorentz])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
Got a couple reports of a mysterious box appearing in the browser UI, containing text with a crashID... "there was a frame on the right, with a width of about 150-200px, white apart a crash ID into it. it was starting under bookmarks toolbar and going down till the statusbar. tabbar was on the left of this frame that was not closeable nor resizeable nor whatever."
I think what's happening is that the iframe added by CrashSubmit.jsm needs to hidden (display:none or make is 0x0). I don't see that being done anywhere. Mak also noted that this iframe seems to persist... Not sure if it was just really really slow to submit, or there's a case where it fails to get removed (looks like the code does at least attempt to remove the iframe after submission).
Comment 1•15 years ago
|
||
I don't think it's just a slow submission - I had it appear yesterday, and it persisted overnight. Unless it's a timeout, i guess.
Comment 2•15 years ago
|
||
Bleh, it ought to be removed, dunno why it wouldn't be. Also, I wish we had proper APIs so this didn't have to use an iframe.
Comment 3•15 years ago
|
||
This is what one of my windows looks like right now, thanks to this bug and the #1 topcrash bug on Linux.
Comment 4•15 years ago
|
||
Heh. I wonder if the CrashSubmit code is failing here because it expects to be operating on HTML? about:crashes is XHTML, but clearly when used in browser.xul it's operating on XUL.
Assuming the webprogresslistener gets all the way to STATE_STOP, it should remove the iframe no matter what:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/CrashSubmit.jsm#268
Might be interesting to stick some extra debug statements in there to find out what's happening.
Assignee | ||
Comment 5•15 years ago
|
||
Is anyone seeing this on Windows?
Comment 6•15 years ago
|
||
Error: this.iframe.docShell.removeProgressListener is not a function
Source File: file:///builds/minefield-x86/firefox/modules/CrashSubmit.jsm
Line: 269
This also seemingly prevents the plugin-crash UI from actually showing.
Comment 7•15 years ago
|
||
Note, #testday participants also say that whenever the crash reporter is enabled on Linux they never see the plugin-crashed UI. The iframe being stuck visible is only an occasional problem beyond that.
Comment 8•15 years ago
|
||
more details: the first time I go to http://flashcrash.dempsky.org/ the iframe doesn't appear, but no plugin-crash UI. I hit refresh, and the iframe appears with the error console message about .removeProgressListener.
Comment 9•15 years ago
|
||
The code explicitly creates a xul:iframe:
http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/CrashSubmit.jsm#312
I don't know why that would fail.
Comment 10•15 years ago
|
||
(In reply to comment #5)
> Is anyone seeing this on Windows?
Yes, Windows 7 here.
Assignee | ||
Comment 11•15 years ago
|
||
I suspect some (all?) cases of the plugin-crashed UI not being shown are either bug 544550 or bug 545686.
Assignee | ||
Comment 12•15 years ago
|
||
I was able to reproduce the sticky iframe by unplugging my network cable, the form submission fails after a few seconds and the frame ends up holding an error page. [Note to self: also need to set MOZ_CRASHREPORTER=1 to trigger submission in own builds.]
I got other errors if I styled the iframe with display:none (presumably that's bad to do for an iframe with content you're trying to manipulate?). So, instead, I set width=0 plus a minWidth override for the default iframe style.
The error that was being thrown in some cases is fixed by adding a QI. I guess the error page results in a new docshell (which hasn't been QI'd to this interface yet?)...
Assignee: nobody → dolske
Attachment #426642 -
Attachment is obsolete: true
Attachment #427047 -
Flags: review?(ted.mielczarek)
Updated•15 years ago
|
Blocks: LorentzAlpha
Comment 13•15 years ago
|
||
Comment on attachment 427047 [details] [diff] [review]
Patch v.1
>diff --git a/toolkit/crashreporter/CrashSubmit.jsm b/toolkit/crashreporter/CrashSubmit.jsm
>--- a/toolkit/crashreporter/CrashSubmit.jsm
>+++ b/toolkit/crashreporter/CrashSubmit.jsm
>@@ -261,16 +261,17 @@ Submitter.prototype = {
> aIID.equals(Ci.nsISupports))
> return this;
> throw Components.results.NS_NOINTERFACE;
> },
>
> onStateChange: function(aWebProgress, aRequest, aFlag, aStatus)
> {
> if(aFlag & STATE_STOP) {
>+ this.iframe.docShell.QueryInterface(Ci.nsIWebProgress);
> this.iframe.docShell.removeProgressListener(this);
>
> // check general request status first
> if (!Components.isSuccessCode(aStatus)) {
> this.element.removeChild(this.iframe);
> if (this.errorCallback) {
> this.errorCallback(this.id);
> }
>@@ -306,16 +307,18 @@ Submitter.prototype = {
> if (!dump.exists() || !extra.exists()) {
> this.cleanup();
> return false;
> }
> this.dump = dump;
> this.extra = extra;
> let iframe = this.document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "iframe");
> iframe.setAttribute("type", "content");
>+ iframe.style.width = 0;
>+ iframe.style.minWidth= 0;
nit: spaces around equals sign.
Looks fine otherwise, r=me
Attachment #427047 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 14•15 years ago
|
||
Attachment #427047 -
Attachment is obsolete: true
Assignee | ||
Comment 15•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Comment 16•15 years ago
|
||
Whiteboard: [fixed-lorentz]
Comment 17•15 years ago
|
||
Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed
Comment 18•15 years ago
|
||
Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430
status1.9.2:
--- → .4-fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•