Closed Bug 940323 Opened 7 years ago Closed 6 years ago

Move the strictMode flag from ContextOptions to RuntimeOptions

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: ejpbruel, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
This probably isn't going to be looked at until bug 939562 lands. Marking the dep so I'll see it fly by when that bug lands.
Depends on: 939562
Attachment #8455487 - Flags: review?(jdemooij)
Comment on attachment 8455487 [details] [diff] [review]
Mode strictMode to RuntimeOptions. v1

Review of attachment 8455487 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCComponents.cpp
@@ +3243,5 @@
>      }
>  
>  GENERATE_JSCONTEXTOPTION_GETTER_SETTER(Strict, extraWarnings, setExtraWarnings)
>  GENERATE_JSRUNTIMEOPTION_GETTER_SETTER(Werror, werror, setWerror)
> +GENERATE_JSRUNTIMEOPTION_GETTER_SETTER(Strict_mode, strictMode, setStrictMode)

Could this break code/websites, due to some addon toggling "use strict" for the entire runtime? I did some grepping and I don't think we set Cu.strict_mode ourselves. If you think this is okay to change I'll trust your judgement.
Attachment #8455487 - Flags: review?(jdemooij) → review+
(In reply to Jan de Mooij [:jandem] from comment #3)
> Comment on attachment 8455487 [details] [diff] [review]
> Mode strictMode to RuntimeOptions. v1
> 
> Review of attachment 8455487 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/xpconnect/src/XPCComponents.cpp
> @@ +3243,5 @@
> >      }
> >  
> >  GENERATE_JSCONTEXTOPTION_GETTER_SETTER(Strict, extraWarnings, setExtraWarnings)
> >  GENERATE_JSRUNTIMEOPTION_GETTER_SETTER(Werror, werror, setWerror)
> > +GENERATE_JSRUNTIMEOPTION_GETTER_SETTER(Strict_mode, strictMode, setStrictMode)
> 
> Could this break code/websites, due to some addon toggling "use strict" for
> the entire runtime? I did some grepping and I don't think we set
> Cu.strict_mode ourselves. If you think this is okay to change I'll trust
> your judgement.

Yeah. In general, the old model of having them on the JSContext wasn't really all that fool-proof. That definitely presents a risk, but we don't really have an alternative way to preserve the same semantics. If we encounter breakage on beta, we can just make this flag a no-op (though Jesse might not like that).
https://hg.mozilla.org/mozilla-central/rev/d6289d614a70
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.