Closed Bug 1207491 Opened 9 years ago Closed 9 years ago

Remove use of expression closure from browser/components/.

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: arai, Unassigned)

References

Details

Attachments

(20 files)

40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
40 bytes, text/x-review-board-request
Gijs
: review+
Details
Need to replace non-standard expression closure with one of:
  * function declaration
  * function expression
  * arrow function
before fixing bug 1083458.

converting rules are following:
  * function declaration
    add `return` and braces
  * standalone named function expression
    add `return` and braces
  * standalone anonymous function expression contans and receives `this` (Array.filter, bind, etc)
    convert to arrow function, and remove code passing |this|
  * standalone anonymous function expression contans no `this`
    convert to arrow function
  * property with anonymous function expression, contains `this`
    add `return` and braces
  * property with anonymous function expression, contains no `this`, short body
    convert to arrow function
  * property with anonymous function expression, contains no `this`, long body
    add `return` and braces
  * property with named function expression
    add `return` and braces
  * getter property
    add `return` and braces
  * setter property
    add braces

Since there are a lot of patches, separated into 8 bugs, each bug corresponds to one of following directories:
  * browser/, except browser/components/.
  * browser/components/.
  * dom/.
  * layout/.
  * services/.
  * toolkit/, except toolkit/components/.
  * toolkit/components/.
  * b2g/, chrome/, docshell/, mobiles/, modules/, netwerk/, parser/, security/, storage/, testing/, webapprt/, widget/, xpcom/
(not yet touched addon-sdk)

I have draft patches, will post them (may take some time to prepare and post).
Bug 1207491 - Part 1: Remove use of expression closure from browser/components/customizableui/CustomizableUI.jsm. r?Gijs
Attachment #8664916 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 2: Remove use of expression closure from browser/components/customizableui/PanelWideWidgetTracker.jsm. r?Gijs
Attachment #8664917 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 3: Remove use of expression closure from browser/components/customizableui/content/. r?Gijs
Attachment #8664918 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 4: Remove use of expression closure from browser/components/customizableui/test/. r?Gijs
Attachment #8664919 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 5: Remove use of expression closure from browser/components/distribution.js. r?Gijs
Attachment #8664920 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 6: Remove use of expression closure from browser/components/downloads/. r?Gijs
Attachment #8664921 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 7: Remove use of expression closure from browser/components/migration/. r?Gijs
Attachment #8664922 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 8: Remove use of expression closure from browser/components/nsBrowserContentHandler.js. r?Gijs
Attachment #8664923 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 9: Remove use of expression closure from browser/components/places/PlacesUIUtils.jsm. r?Gijs
Attachment #8664924 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 10: Remove use of expression closure from browser/components/places/content/. r?Gijs
Attachment #8664925 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 11: Remove use of expression closure from browser/components/places/tests/. r?Gijs
Attachment #8664926 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 12: Remove use of expression closure from browser/components/pocket/. r?Gijs
Attachment #8664927 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 13: Remove use of expression closure from browser/components/preferences/. r?Gijs
Attachment #8664928 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 14: Remove use of expression closure from browser/components/privatebrowsing/. r?Gijs
Attachment #8664929 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 15: Remove use of expression closure from browser/components/search/. r?Gijs
Attachment #8664930 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 16: Remove use of expression closure from browser/components/sessionstore/. r?Gijs
Attachment #8664931 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 17: Remove use of expression closure from browser/components/shell/. r?Gijs
Attachment #8664932 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 18: Remove use of expression closure from browser/components/tabview/. r?Gijs
Attachment #8664933 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 19: Remove use of expression closure from browser/components/tabview/test/. r?Gijs
Attachment #8664934 - Flags: review?(gijskruitbosch+bugs)
Bug 1207491 - Part 20: Remove use of expression closure from browser/components/translation/. r?Gijs
Attachment #8664935 - Flags: review?(gijskruitbosch+bugs)
https://reviewboard.mozilla.org/r/20057/#review18069

::: browser/components/customizableui/CustomizableUI.jsm:2740
(Diff revision 1)
> -  get AREA_PANEL() "PanelUI-contents",
> +  get AREA_PANEL() {
> +    return "PanelUI-contents";
> +  },

this.CustomizableUI is frozen, too, so I think it's fine to just make these properties, too.

::: browser/components/migration/360seProfileMigrator.js:81
(Diff revision 1)
> -  function toHexString(charCode)
> -    ("0" + charCode.toString(16)).slice(-2);
> +  function toHexString(charCode) {
> +    return ("0" + charCode.toString(16)).slice(-2);
> +  }

This or:

let toHexString = charCode => ("0" + charCode.toString(16)).slice(-2);

::: browser/components/migration/IEProfileMigrator.js:358
(Diff revision 1)
> -    function yesNoToBoolean(v) v == "yes";
> +    function yesNoToBoolean(v) {
> +      return v == "yes";
> +    }

here too, let yesNoToBoolean = v => v == "yes";

::: browser/components/preferences/translation.js:28
(Diff revision 1)
> -  get boxObject() this._tree.treeBoxObject,
> -  get isEmpty() !this._data.length,
> -  get hasSelection() this.selection.count > 0,
> +  get boxObject() {
> +    return this._tree.treeBoxObject;
> +  },
> +  get boxObject() {
> +    return this._tree.treeBoxObject;
> +  },

Seems like this is now duplicated? Not sure what went wrong here...
Comment on attachment 8664917 [details]
MozReview Request: Bug 1207491 - Part 2: Remove use of expression closure from browser/components/customizableui/PanelWideWidgetTracker.jsm. r?Gijs

https://reviewboard.mozilla.org/r/20061/#review18139
Attachment #8664917 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664916 [details]
MozReview Request: Bug 1207491 - Part 1: Remove use of expression closure from browser/components/customizableui/CustomizableUI.jsm. r?Gijs

https://reviewboard.mozilla.org/r/20059/#review18141
Attachment #8664916 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664918 [details]
MozReview Request: Bug 1207491 - Part 3: Remove use of expression closure from browser/components/customizableui/content/. r?Gijs

https://reviewboard.mozilla.org/r/20063/#review18143
Attachment #8664918 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664919 [details]
MozReview Request: Bug 1207491 - Part 4: Remove use of expression closure from browser/components/customizableui/test/. r?Gijs

https://reviewboard.mozilla.org/r/20065/#review18145
Attachment #8664919 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8664920 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664920 [details]
MozReview Request: Bug 1207491 - Part 5: Remove use of expression closure from browser/components/distribution.js. r?Gijs

https://reviewboard.mozilla.org/r/20067/#review18147
Attachment #8664928 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8664928 [details]
MozReview Request: Bug 1207491 - Part 13: Remove use of expression closure from browser/components/preferences/. r?Gijs

https://reviewboard.mozilla.org/r/20083/#review18149
Comment on attachment 8664921 [details]
MozReview Request: Bug 1207491 - Part 6: Remove use of expression closure from browser/components/downloads/. r?Gijs

https://reviewboard.mozilla.org/r/20069/#review18153
Attachment #8664921 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664922 [details]
MozReview Request: Bug 1207491 - Part 7: Remove use of expression closure from browser/components/migration/. r?Gijs

https://reviewboard.mozilla.org/r/20071/#review18157
Attachment #8664922 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664923 [details]
MozReview Request: Bug 1207491 - Part 8: Remove use of expression closure from browser/components/nsBrowserContentHandler.js. r?Gijs

https://reviewboard.mozilla.org/r/20073/#review18159
Attachment #8664923 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664924 [details]
MozReview Request: Bug 1207491 - Part 9: Remove use of expression closure from browser/components/places/PlacesUIUtils.jsm. r?Gijs

https://reviewboard.mozilla.org/r/20075/#review18161
Attachment #8664924 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8664925 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664925 [details]
MozReview Request: Bug 1207491 - Part 10: Remove use of expression closure from browser/components/places/content/. r?Gijs

https://reviewboard.mozilla.org/r/20077/#review18163
Comment on attachment 8664926 [details]
MozReview Request: Bug 1207491 - Part 11: Remove use of expression closure from browser/components/places/tests/. r?Gijs

https://reviewboard.mozilla.org/r/20079/#review18165
Attachment #8664926 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664927 [details]
MozReview Request: Bug 1207491 - Part 12: Remove use of expression closure from browser/components/pocket/. r?Gijs

https://reviewboard.mozilla.org/r/20081/#review18167
Attachment #8664927 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664929 [details]
MozReview Request: Bug 1207491 - Part 14: Remove use of expression closure from browser/components/privatebrowsing/. r?Gijs

https://reviewboard.mozilla.org/r/20085/#review18169
Attachment #8664929 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664930 [details]
MozReview Request: Bug 1207491 - Part 15: Remove use of expression closure from browser/components/search/. r?Gijs

https://reviewboard.mozilla.org/r/20087/#review18171
Attachment #8664930 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664931 [details]
MozReview Request: Bug 1207491 - Part 16: Remove use of expression closure from browser/components/sessionstore/. r?Gijs

https://reviewboard.mozilla.org/r/20089/#review18175
Attachment #8664931 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664932 [details]
MozReview Request: Bug 1207491 - Part 17: Remove use of expression closure from browser/components/shell/. r?Gijs

https://reviewboard.mozilla.org/r/20091/#review18177
Attachment #8664932 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664933 [details]
MozReview Request: Bug 1207491 - Part 18: Remove use of expression closure from browser/components/tabview/. r?Gijs

https://reviewboard.mozilla.org/r/20093/#review18179
Attachment #8664933 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664934 [details]
MozReview Request: Bug 1207491 - Part 19: Remove use of expression closure from browser/components/tabview/test/. r?Gijs

https://reviewboard.mozilla.org/r/20095/#review18181
Attachment #8664934 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664935 [details]
MozReview Request: Bug 1207491 - Part 20: Remove use of expression closure from browser/components/translation/. r?Gijs

https://reviewboard.mozilla.org/r/20097/#review18183
Attachment #8664935 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8664928 [details]
MozReview Request: Bug 1207491 - Part 13: Remove use of expression closure from browser/components/preferences/. r?Gijs

Per discussion on IRC, r+ with the duplication fixed
Attachment #8664928 - Flags: review+
(In reply to Tooru Fujisawa [:arai] from comment #0)
>   * property with anonymous function expression, contains `this`
>     add `return` and braces

Just to be sure, you know `arguments` is also lexical right? See bug 889158 and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#Lexical_arguments
You need to log in before you can comment on or make changes to this bug.