1125498 - Use for..of iteration instead of Array.forEach in browser-gestureSupport.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

Look for "forEach" (without quotes) in browser/base/content/browser-gestureSupport.js. There should be two matches that can be simplified as follows...

instead of:

  x.forEach(function (y) {
    ...;
  });

or:

  x.forEach(function (y) ..., this);

we can do:

  for (let y of x) {
    ...;
  }

Comment 1

[Abhishek Bhattacharya [:babhishek21]]

On it.

Comment 2

[Abhishek Bhattacharya [:babhishek21]]

Attachment: bug_1125498: Initial Patch.

So fixed two occurrences.

Heading over to bug 1125497

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8554177 [details] [diff] [review]
bug_1125498: Initial Patch.

>-    gestureEvents.forEach(function (event) addRemove("Moz" + event, this, true),
>-                          this);
>+	for(let event of gestureEvents) {
>+		addRemove("Moz" + event, this, true);
>+	}

Please use spaces instead of tabs and put a space between 'for' and '('.

>-    ["shift", "alt", "ctrl", "meta"].forEach(function (key) {
>-      if (aEvent[key + "Key"])
>-        keyCombos.push(key);
>-    });
>+	let pressedKeys = ["shift", "alt", "ctrl", "meta"];
>+	for(let key of pressedKeys) {
>+		if (aEvent[key + "Key"])
>+	        keyCombos.push(key);
>+	}

Same here. Also, no need to introduce a variable, the array literal can go right into the for-loop head.

Comment 4

[Abhishek Bhattacharya [:babhishek21]]

Attachment: bug_1125498: rev_1 (Better code style)

Rev_1: Better style.

BTW, can you  please assign this bug to me.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8554269 [details] [diff] [review]
bug_1125498: rev_1 (Better code style)

There are still tabs in here, please use spaces instead.

Comment 6

[Abhishek Bhattacharya [:babhishek21]]

Attachment: bug_1125498: rev_2 (Fixed TABS/spaces)

So I have tried fixing astray TABS/spaces. However my text-editor (Sublime Text) "reflows" indents for visual fidelity. So sometimes two-space indents and four-space indents look the same. I have tried on my part. If you find additional problems, do tell.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8555972 [details] [diff] [review]
bug_1125498: rev_2 (Fixed TABS/spaces)

>-    ["shift", "alt", "ctrl", "meta"].forEach(function (key) {
>+    for (let key of ["shift", "alt", "ctrl", "meta"]) {
>       if (aEvent[key + "Key"])
>-        keyCombos.push(key);
>-    });
>+      keyCombos.push(key);
>+    }

keyCombos.push(key); should be indented further like it was originally. Everything else looks good now!

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8555972 [details] [diff] [review]
bug_1125498: rev_2 (Fixed TABS/spaces)

Landed with comment 7 addressed:

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

Comment 9

[Abhishek Bhattacharya [:babhishek21]]

(In reply to Dão Gottwald [:dao] (unavailable in February) from comment #8)
> Comment on attachment 8555972 [details] [diff] [review]
> bug_1125498: rev_2 (Fixed TABS/spaces)
> 
> Landed with comment 7 addressed:
> 
> https://hg.mozilla.org/integration/fx-team/rev/d4f795acf79c

Thanks!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d4f795acf79c