Closed Bug 737022 Opened 8 years ago Closed 8 years ago

Re-enable strict warnings while parsing XUL

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: neil, Assigned: neil)

References

Details

(Keywords: regression)

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #385872 +++

Bug 383365 enabled strict warnings in debug builds by default, for both chrome and content.

Bug 385872, while trying to turn off strict warnings in content in debug builds, managed to completely disable strict warnings while parsing XUL, because the call to JSOptionChangedCallback was moved from nsJSContext::nsJSContext (where everyone gets it) to nsJSContext::InitClasses (where only HTML gets it).
Attached patch Proposed patchSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #607215 - Flags: review?(jst)
Blocks: 371174
Summary: Strict warnings should only be enabled for chrome by default (in debug builds) → Re-enable strict warnings while parsing XUL
What types of warnings does this enable? I see many warnings in debug builds that aren't seen in normal builds.
Like 'reference to undefined property' or 'variable redeclaration'.
I mean in the Error console.
Is this bug about something else?
(In reply to :aceman from comment #2)
> What types of warnings does this enable? I see many warnings in debug builds
> that aren't seen in normal builds.
> Like 'reference to undefined property' or 'variable redeclaration'.
This enables warnings generated while parsing XUL scripts. You may already be able to trigger those warnings in JS components or modules though; I don't know how their loaders work. Some of the warnings that may be involved: function does not always return a value; redeclaration of variable; test for equality mistyped as assignment? variable redeclares argument; mistyped ; after conditional?
Attachment #607215 - Flags: review?(jst) → review+
Try run for 438f35edbb51 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=438f35edbb51
Results (out of 219 total builds):
    exception: 2
    success: 175
    warnings: 28
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-438f35edbb51
Pushed changeset 5c13fce74f83 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Mano moved the setting of the context options from InitContext to InitClasses so that it would have a chance to affect global windows after the context had been given a script global object; InitContext is too early for that...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch More fixesSplinter Review
* Now sets the context options in InitClasses too where we have a window that we can check for chromeness.
* Tweaks checks for content windows so that they don't include XBL/XUL script prototype objects, which will use the chrome window preferences. (XUL actually gives us the script prototype object before it calls InitContext so in theory I could check for chrome XUL, but I hear nobody supports content XUL these days.)
Attachment #609721 - Flags: review?(jst)
You can still whitelist content XUL via the permissions manager.
Attachment #609721 - Flags: review?(jst) → review+
Pushed changeset fb23c30e3d60 to mozilla-central.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Depends on: 738589
No longer depends on: 738589
You need to log in before you can comment on or make changes to this bug.