Re-enable strict warnings while parsing XUL

RESOLVED FIXED in mozilla14

Status

()

Core
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

({regression})

Trunk
mozilla14
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
+++ 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).
(Assignee)

Comment 1

5 years ago
Created attachment 607215 [details] [diff] [review]
Proposed patch
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

Comment 2

5 years ago
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'.

Comment 3

5 years ago
I mean in the Error console.
Is this bug about something else?
(Assignee)

Comment 4

5 years ago
(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?

Updated

5 years ago
Attachment #607215 - Flags: review?(jst) → review+

Comment 5

5 years ago
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
(Assignee)

Comment 6

5 years ago
Pushed changeset 5c13fce74f83 to mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
(Assignee)

Comment 7

5 years ago
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 → ---
(Assignee)

Comment 8

5 years ago
Created attachment 609721 [details] [diff] [review]
More fixes

* 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)

Comment 9

5 years ago
You can still whitelist content XUL via the permissions manager.

Updated

5 years ago
Attachment #609721 - Flags: review?(jst) → review+
(Assignee)

Comment 10

5 years ago
Pushed changeset fb23c30e3d60 to mozilla-central.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 738589
(Assignee)

Updated

5 years ago
No longer depends on: 738589
You need to log in before you can comment on or make changes to this bug.