updateStatusBar not defined in gSelectionListener() in viewSource.js

RESOLVED FIXED in mozilla1.9.2a1

Status

()

defect
--
minor
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: matt, Assigned: philor)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.2a1
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090223 Minefield/3.2a1pre ID:20090223020535

I intermittently get the following JavaScript error after closing the view source window (which I always open by clicking on the source link of an error in the error console):

Error: updateStatusBar is not defined
Source  chrome://global/content/viewSource.js
Line: 29

Which refers to this line in gSelectionListener():

      this.timeout = setTimeout(updateStatusBar, 100);
You don't, however, *have* to open it from the console to have it firing away after the window is gone, source of a web page will work just as well.
OS: Linux → All
Hardware: x86 → All
Version: unspecified → Trunk
Does this apply to 1.9.1?
Flags: blocking1.9.2?
Posted patch Fix (obsolete) — Splinter Review
Yes, it applies to 1.9.1, where it's giving me a rash in Thunderbird from seeing one error, wanting to see where it was, getting another error for my troubles.
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Attachment #375955 - Flags: review?(gavin.sharp)
Attachment #375955 - Attachment is obsolete: true
Attachment #375955 - Flags: review?(gavin.sharp)
Comment on attachment 375955 [details] [diff] [review]
Fix

Oops, just because I usually only load one document in a viewsource window, doesn't mean it's not possible to have a whole bunch of listeners stacked up waiting for their final fling.
Posted patch Better fixSplinter Review
Slightly tacky, but considerably nicer than a try/catch swallowing real errors just to skip that first unload.
Attachment #376019 - Flags: review?(gavin.sharp)
Attachment #376019 - Flags: review?(gavin.sharp) → review+
Is this related to bug 490071 and/or bug 471126 at all?
Yeah, I'd say bug 490071 is almost certainly the answer to my "am I working around something that should be fixed instead?" question. So my next question is, should I check this in anyway? Is one or the other of us more likely to make it to 1.9.1?
You should check this in anyway; you can't know when GC is going to happen.
http://hg.mozilla.org/mozilla-central/rev/9f9b05760fff
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
backed out to see if this fixes recent orange

http://hg.mozilla.org/mozilla-central/rev/d525ab89ac02
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The orange in question, for what it's worth, was miscellaneous errors (sometimes timeouts, sometimes test failures, sometimes both) always preceded by:

*** 13019 INFO Error: Unable to restore focus, expect failures and timeouts.

So far, it looks like the backout may well have fixed it, although we haven't had enough cycles to be sure.
Huh. And does someone have a theory for *how* it might fix it? Keeping in mind that all I touched was viewSource.js, which is only included by viewSource.xul and viewPartialSource.xul, and none of those three files are mentioned directly in any test code, and no test code calls gViewSourceUtils.viewSource(), or .openInInternalViewer(), I'm a bit at a loss as to how I could be causing test failures.

Is the theory that... um... we have test code that clicks on a console-error-source link? Or that makes a windowtype="navigator:view-source" (programmatically, and by assembling the pieces, since nothing directly includes the string "navigator:view-source") and then drops something on it?
I think this is clear to reland.
Keywords: checkin-needed
Relanded: http://hg.mozilla.org/mozilla-central/rev/53351b69bbbd
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Comment on attachment 376019 [details] [diff] [review]
Better fix

Drivers: I'm ambivalent about the wisdom of taking this untested patch to untested code. The reward (besides the intrinsic reward of getting rid of a 100ms "leak") is skipping half a dozen duplicates when people fail to search for fixed bugs, and skipping a couple dozen "and the error console says 'updateStatusBar is undefined'" "that's meaningless and unrelated" exchanges in bugs about other things. The risk is that some way of getting content in view-source that I can't imagine (and thus wouldn't test even if this had tests) would cause an unload but not a load, and we'd wind up losing the statusbar display of line and column number, unless and until something did load in the window.
Attachment #376019 - Flags: approval1.9.1?
Comment on attachment 376019 [details] [diff] [review]
Better fix

a191=beltzner
Attachment #376019 - Flags: approval1.9.1? → approval1.9.1+
Flags: blocking1.9.2? → blocking1.9.2+
You need to log in before you can comment on or make changes to this bug.