Closed Bug 673104 Opened 10 years ago Closed 10 years ago

Use ES5 strict mode for Panorama

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 8

People

(Reporter: ttaubert, Assigned: ttaubert)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
OS: Linux → All
Hardware: x86_64 → All
Attached patch patch v1 (obsolete) — Splinter Review
Attachment #547410 - Flags: feedback?(raymond)
Comment on attachment 547410 [details] [diff] [review]
patch v1

>--- a/browser/base/content/tabview/modules/utils.jsm
>+++ b/browser/base/content/tabview/modules/utils.jsm

>+    function tanh(x) {
>+      var e = Math.exp(x);
>+      return (e - 1/e) / (e + 1/e);
>+    }
>+
>     if (smooth) {
>       // The ease function ".5+.5*Math.tanh(4*x-2)" is a pretty
>       // little graph. It goes from near 0 at x=0 to near 1 at x=1
>       // smoothly and beautifully.
>       // http://www.wolframalpha.com/input/?i=.5+%2B+.5+*+tanh%28%284+*+x%29+-+2%29
>-      function tanh(x) {
>-        var e = Math.exp(x);
>-        return (e - 1/e) / (e + 1/e);
>-      }
>       return .5 - .5 * tanh(2 - 4 * proportion);
>     }

>--- a/browser/base/content/tabview/search.js
>+++ b/browser/base/content/tabview/search.js

>+  function dispatchTabViewSearchEnabledEvent() {
>+    let newEvent = document.createEvent("Events");
>+    newEvent.initEvent("tabviewsearchenabled", false, false);
>+    dispatchEvent(newEvent);
>+  }
>+
>   if (!isSearchEnabled()) {
>     $searchShade.show();
>     $search.show();
> 
> #ifdef XP_MACOSX
>     UI.setTitlebarColors({active: "#717171", inactive: "#EDEDED"});
> #endif
> 
>     // NOTE: when this function is called by keydown handler, next keypress
>     // event or composition events of IME will be fired on the focused editor.
>-
>-    function dispatchTabViewSearchEnabledEvent() {
>-      let newEvent = document.createEvent("Events");
>-      newEvent.initEvent("tabviewsearchenabled", false, false);
>-      dispatchEvent(newEvent);
>-    };

That's annoying. What rule enforces this?
Again, annoying. Does let foo = function ... work?
Attached patch patch v2Splinter Review
(In reply to comment #4)
> Again, annoying. Does let foo = function ... work?

Yep, it does.
Attachment #547410 - Attachment is obsolete: true
Attachment #547523 - Flags: feedback?(raymond)
Attachment #547410 - Flags: feedback?(raymond)
Attachment #547523 - Flags: feedback?(raymond) → review?(dao)
Comment on attachment 547523 [details] [diff] [review]
patch v2

> function observer(subject, topic, data) {
>   switch (topic) {
>     case "domwindowopened":
>-      subject.addEventListener("load", function() {
>-        subject.removeEventListener("load", arguments.callee, false);
>+      subject.addEventListener("load", function onLoad() {
>+        subject.removeEventListener("load", onLoad, false);

I wonder where this makes onLoad available, exactly, and why this doesn't bother strict mode.
Attachment #547523 - Flags: review?(dao) → review+
(In reply to comment #6)
> I wonder where this makes onLoad available, exactly, and why this doesn't
> bother strict mode.

In this case it's a function expression (not a function definition) and so the name of it is bound locally inside the function, but not outside.
Depends on: 673804
http://hg.mozilla.org/mozilla-central/rev/f79f1ea87e2c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 8
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.