1122433 - Remove use of expression closures from AboutHome.jsm

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

+++ This bug was initially created as a clone of Bug #1122356 +++

Expression closures (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Expression_closures) are a nonstandard language extension we'd like to remove from SpiderMonkey.

In AboutHome.jsm, this only affects the snippetsVersion getter as far as I can see:

http://hg.mozilla.org/mozilla-central/annotate/cac6192956ab/browser/modules/AboutHome.jsm#l30

The general pattern for getters is that:

get x() y,

needs to be replaced with:

get x() {
  return y;
},

Comment 1

[Vikram Jadhav]

Hi again,
Can i work on this bug?
Thanks.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Vikram Jadhav from comment #1)
> Hi again,
> Can i work on this bug?
> Thanks.

Since you're already fixing a very similar bug, I'd suggest trying something different now, like bug 1122430.

Comment 3

[Vaibhav Bhosale]

Hey
I am new to bugzilla
Can you help me to start off?

Comment 4

[Dão Gottwald [:dao]]

(In reply to Vaibhav Bhosale from comment #3)
> Hey
> I am new to bugzilla
> Can you help me to start off?

Hi. Have you read https://developer.mozilla.org/en-US/docs/Introduction?

Comment 5

[Kushan Joshi [:kepta]]

Hi,
Can I work on this?

Comment 6

[Dão Gottwald [:dao]]

(In reply to 0o3ko0 from comment #5)
> Hi,
> Can I work on this?

Feel free to, as long as this bug isn't assigned and nobody has attached a patch.

Comment 7

[shreyas]

Attachment: part 1

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8550850 [details] [diff] [review]
part 1

Looks about right, but indentation is off (and should use spaces only, no tabs) and there should be a semicolon after the return statement...

Comment 9

[shreyas]

Attachment: ref1-Replaces expression closure

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 8550862 [details] [diff] [review]
ref1-Replaces expression closure

> this.AboutHomeUtils = {
>   get snippetsVersion() {
>-	return STARTPAGE_VERSION
>+        return STARTPAGE_VERSION;
>     },

The line with the return statement should be indented with 4 spaces and } with 2 spaces. Also, please make sure your diff is against the repository head rather than on top of your previous patch.

Comment 11

[shreyas]

Attachment: bug1122433_expression_closurefixes2.diff

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8550904 [details] [diff] [review]
bug1122433_expression_closurefixes2.diff

Something went wrong here... this file is basically empty, there's no patch

Comment 13

[shreyas]

The same copy of the diff file is getting generated every time I run the diff.

Comment 14

[Dão Gottwald [:dao]]

I suspect you've committed your changes and diff only cares about uncommitted changes. What do you get when running hg out?

Comment 15

[shreyas]

comparing with https://hg.mozilla.org/mozilla-central/
searching for changes
changeset:   224093:22dd808317ca
user:        shreyas <[e-mail]>
date:        Sun Jan 18 11:23:06 2015 +0530
summary:     [mq]: shreyas.patch

changeset:   224094:3e0de4967e79
user:        shreyas <[e-mail]>
date:        Sun Jan 18 11:23:45 2015 +0530
summary:     [mq]: 1122433.patch

changeset:   224095:aeab5d86be38
tag:         qparent
user:        shreyas <[e-mail]>
date:        Sun Jan 18 12:36:59 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

changeset:   224096:eeb897ff519b
tag:         bug1122433_expression_closurefixes.diff
tag:         qbase
user:        shreyas <[e-mail]>
date:        Sun Jan 18 15:04:49 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

changeset:   224436:3a61f863d15e
tag:         bug1122433_expression_closurefixes1.diff
parent:      224096:eeb897ff519b
user:        shreyas <[e-mail]>
date:        Sun Jan 18 21:36:03 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

changeset:   224437:7a5852c22869
tag:         bug1122433_expression_closurefixes2.diff
user:        shreyas <[e-mail]>
date:        Sun Jan 18 21:39:24 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

changeset:   224438:0d117974a674
tag:         bug1122433_expression_closurefixes3.diff
user:        shreyas <[e-mail]>
date:        Sun Jan 18 21:46:21 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

changeset:   224439:9d5b3039a9ab
tag:         bug1122433_expression_closurefixes4.diff
tag:         qtip
tag:         tip
user:        shreyas <[e-mail]>
date:        Sun Jan 18 21:58:08 2015 +0530
summary:     Bug 1122433 - Expression closure replaced in Abouthome.jsm

Comment 16

[shreyas]

Attachment: part 2 (addressed review comments)

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 8551268 [details] [diff] [review]
part 2 (addressed review comments)

This is still on top of attachment 8550850 [details] [diff] [review]. I'll combine these two patches and get this landed :)

Comment 18

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/5161e9b47a3c

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5161e9b47a3c