Closed Bug 329882 Opened 14 years ago Closed 11 years ago

bc has no properties console.js

Categories

(Toolkit Graveyard :: Error Console, defect)

x86
Windows XP
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: bugzilla, Assigned: mkaply)

References

Details

Attachments

(1 file, 1 obsolete file)

using latest nightly build I'm seeing:

Error: bc has no properties
Source File: chrome://global/content/console.js
Line: 61
/me resents the implication. ;-)
Henrik: Have you ever seen this again? Any STR?

One known way to get this error is to install Console², apply some filters, then uninstall the extension and reopen the original Error Console. Clicking one of the four filter buttons fixes that (non-)issue.
Henrik: Without any answer to comment #2, there isn't much we can do here.

Please reopen this bug, if you can indicate Steps to Reproduce (if possible on a clean profile).
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
Duplicate of this bug: 416869
What can't we just add an if statement since we know where the problem occurs?
> What can't we just add an if statement since we know where the problem occurs?

You know the drill. Attach a patch, ask for review. etc.
OK, so the bug here has to do with the console mode being in a strange stage.

Instead of the mode being "All", the mode is "Errors Warnings Messages"

So when the code is executed:

var bc = document.getElementById("Console:mode" + aMode);

It's trying to get the element:

"Console:modeErrors Warnings Messages"

which is obviously wrong.

So the question is how we get a mode set to "Errors Warnings Messages" instead of "All"
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Definitely the fault of Console2.

Because it's XUL files are named the same, it's overwriting XUL data in localstore.rdf.

Console2 uses states for each type of messages (not all or nothing like us). So after running console2, in localstore.rdf we have:

<RDF:Description RDF:about="chrome://global/content/console.xul#ConsoleBox"
                   modeLang="JS CSS XML"
                   modeCtx="Chrome Content"
                   mode="Errors Warnings Messages" />

That mode is what screws us up

That's an invalid mode for the regular JS console.

It sucks that Console2 is doing this, but I think we should have some sort of workaround so that error doesn't pop up.

Incidentally, easiest way to fix the error locally is to just click on All to reset the value.
Attached patch Attempted fix (obsolete) — Splinter Review
If we fail to get bc, default to "All"

The only other thing we could do here is before attempting to get bc, check to see if aMode is one of the four valid values, but that seems like a lot of work to me for an edge case.
Assignee: nobody → mozilla
Status: REOPENED → ASSIGNED
How about:

var bc = document.getElementById("Console:mode" + aMode);
bc? null: bc = document.getElementById("Console:modeAll");
bc.setAttribute("checked", true);
(In reply to comment #10)
Cleanest would be
- var bc = document.getElementById("Console:mode" + aMode);
+ var bc = document.getElementById("Console:mode" + aMode) || document.getElementById("Console:modeAll");

And please add a comment noting why the mode could get invalid at all...

(In reply to comment #8)
> It sucks that Console2 is doing this

Well, it was meant to be so great that you wouldn't ever think of uninstalling it before it would finally get merged into Toolkit proper... ;-)

But anyway: Why add code for an edge case with a simple work-around?
> (In reply to comment #8)
> > It sucks that Console2 is doing this
> 
> Well, it was meant to be so great that you wouldn't ever think of uninstalling
> it before it would finally get merged into Toolkit proper... ;-)

I hope you didn't take offense to my sucks - didn't mean that Console2 sucks :)

And sometimes you have to disable your extensions. That's just life :)

> But anyway: Why add code for an edge case with a simple work-around?

Manly because, if we can help it, browser code shouldn't put errors in the JS console if it can help it. (even if only extension developers will see it)
Attached patch Better patchSplinter Review
I like Simon's code better.
Attachment #302748 - Attachment is obsolete: true
Product: Firefox → Toolkit
Attachment #302795 - Flags: review?(benjamin)
Attachment #302795 - Flags: review?(benjamin) → review+
http://hg.mozilla.org/mozilla-central/rev/2367bee47379
Status: ASSIGNED → RESOLVED
Closed: 13 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.