Closed
Bug 1160105
Opened 9 years ago
Closed 9 years ago
Fix strict mode warnings in protovis-r2.6-modded.js
Categories
(Thunderbird :: Search, defect)
Thunderbird
Search
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 41.0
People
(Reporter: aleth, Assigned: aleth)
Details
Attachments
(1 file, 1 obsolete file)
5.18 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
In faceted search results: JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 59: SyntaxError: test for equality (==) mistyped as assignment (=)? JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 627: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 1003: TypeError: variable t redeclares argument JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 1844: SyntaxError: test for equality (==) mistyped as assignment (=)? JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 3626: SyntaxError: in strict mode code, functions may be declared only at top level or immediately within another function JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 3612: ReferenceError: reference to undefined property d.$panel JavaScript strict warning: chrome://messenger/content/protovis-r2.6-modded.js, line 1858: ReferenceError: reference to undefined property this.events Maybe Bug 517978 is an alternative.
Assignee | ||
Comment 1•9 years ago
|
||
It's "modded" anyway...
Attachment #8599816 -
Flags: review?(acelists)
Comment on attachment 8599816 [details] [diff] [review] protoviswarnings.diff Review of attachment 8599816 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/base/content/protovis-r2.6-modded.js @@ +55,5 @@ > */ > pv.parse = function(js) { // hacky regex support > var re = new RegExp("function(\\s+\\w+)?\\([^)]*\\)\\s*", "mg"), m, i = 0; > var s = ""; > + while ((m = re.exec(js))) { I already remember this (()) hack works, but I do not know why :) Can it be rewritten in a more readable way? Another occurrence is also later on. @@ +623,5 @@ > return (new pv.Color.Hsl(h, s, l, a)).rgb(); > } > case "rgba": > case "rgb": { > + let parse = c => { // either integer or percentage For me "let name = function () { ... }" is more readable. @@ +3624,5 @@ > d.appendChild(c); > } > > /** Returns the computed style for the given element and property. */ > + let css = (e, p) => { let css = function (e, p) {... }
Attachment #8599816 -
Flags: review?(acelists)
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to :aceman from comment #2) > Comment on attachment 8599816 [details] [diff] [review] > > + while ((m = re.exec(js))) { > > I already remember this (()) hack works, but I do not know why :) Can it be > rewritten in a more readable way? Another occurrence is also later on. It works because the brackets make it explicit that the variable assignment is supposed to happen before the cast to boolean (that is needed to evaluate the condition), i.e. it is made clear that the = is not a typo for ==. One could rewrite it as while (!!(m = re.exec(js))) or move the assignment inside the loop, but neither of those two options make it more readable imho. > @@ +623,5 @@ > > return (new pv.Color.Hsl(h, s, l, a)).rgb(); > > } > > case "rgba": > > case "rgb": { > > + let parse = c => { // either integer or percentage > > For me "let name = function () { ... }" is more readable. I made the change, but arrow functions are becoming quite common ;)
Attachment #8600275 -
Flags: review?(acelists)
Assignee | ||
Updated•9 years ago
|
Attachment #8599816 -
Attachment is obsolete: true
Comment on attachment 8600275 [details] [diff] [review] protoviswarnings.diff v2 Review of attachment 8600275 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8600275 -
Flags: review?(acelists) → review+
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Comment 5•9 years ago
|
||
https://hg.mozilla.org/comm-central/rev/35679800b300 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 41.0
You need to log in
before you can comment on or make changes to this bug.
Description
•