Closed Bug 980558 Opened 6 years ago Closed 5 years ago

Enable extraWarnings on safe JS context

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX
mozilla30

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(1 file)

Attached patch extra-warningsSplinter Review
We use the safe JS context to run content scripts. We're not getting extraWarnings for these scripts. These warnings are often very useful since they tell you when you've used a property that doesn't exist (one of the most common JS errors in my experience).

Setting this option generates a few extra warnings at startup about "function does not always return a value", but it's not too severe. I'll file additional bugs to fix the return value stuff.
Attachment #8387111 - Flags: review?(bobbyholley)
Comment on attachment 8387111 [details] [diff] [review]
extra-warnings

r+ for now, but the semantics here will need to change when this flag moves from the JSContext to the JSRuntime (with a per-compartment override) in bug  940305. So I'm not promising to maintain the behavior here, but I don't want to get in your way in the mean time.
Attachment #8387111 - Flags: review?(bobbyholley) → review+
Thanks Wes. Somehow an extra + character snuck in there when I fixed a conflict.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd36c14f0153
Flags: needinfo?(wmccloskey)
https://hg.mozilla.org/mozilla-central/rev/fd36c14f0153
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Bill backed this out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fec954d58b6
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Hopefully we'll end up with some better system for setting this flag only for chrome code.
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → WONTFIX
(In reply to Bill McCloskey (:billm) from comment #7)
> Hopefully we'll end up with some better system for setting this flag only
> for chrome code.

I implemented such a system in my patch for bug 940305. It's just waiting on Kyle's review.
You need to log in before you can comment on or make changes to this bug.