Use switch statement in AsyncTabSwitcher.handleEvent

RESOLVED FIXED in Firefox 61

Status

()

P3
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dao, Assigned: dwlsalmeida, Mentored)

Tracking

({good-first-bug})

Trunk
Firefox 61
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox61 fixed)

Details

(Whiteboard: [lang=js])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Updated

a year ago
status-firefox61: affected → ---
Priority: -- → P3
(Assignee)

Comment 1

a year ago
This seems nice for a very first contribution!

Please assign it to me, I'll try to submitting a patch.

Thanks.
(Reporter)

Comment 2

a year ago
Okay :)
Assignee: nobody → dwlsalmeida
Status: NEW → ASSIGNED
(Assignee)

Comment 4

a year ago
I have a question:

How can I make sure my new code has no obvious mistakes such as syntax errors? All I did was successfully run ./mach build again, is this enough?
(Reporter)

Comment 5

a year ago
(In reply to Daniel de Almeida from comment #4)
> I have a question:
> 
> How can I make sure my new code has no obvious mistakes such as syntax
> errors? All I did was successfully run ./mach build again, is this enough?

./mach eslint browser/modules/AsyncTabSwitcher.jsm
(Assignee)

Comment 6

a year ago
Please check out my second attempt after running eslint
(Assignee)

Comment 7

a year ago
Attachment #8961806 - Attachment is obsolete: true
Attachment #8961828 - Attachment is obsolete: true
(Reporter)

Comment 8

a year ago
Comment on attachment 8961829 [details] [diff] [review]
(third attempt) 0001-Bug-1447950-use-a-switch-statement-in-place-of-chain.patch

>+    switch (event.type) {
>+
>+      case "MozLayerTreeReady":
>+        this.onLayersReady(event.originalTarget);
>+        break;
>+
>+      case "MozAfterPaint":
>+        this.onPaint();
>+        break;
>+
>+      case "MozLayerTreeCleared":
>+        this.onLayersCleared(event.originalTarget);
>+        break;
>+
>+      case "TabRemotenessChange":
>+        this.onRemotenessChange(event.target);
>+        break;
>+
>+      case "sizemodechange":
>+      case "occlusionstatechange":
>+        this.onSizeModeOrOcclusionStateChange();
>+        break;
>+
>+      case "SwapDocShells":
>+        this.onSwapDocShells(event.originalTarget, event.detail);
>+        break;
>+
>+
>+      case "EndSwapDocShells":
>+        this.onEndSwapDocShells(event.originalTarget, event.detail);
>+        break;
>+
>     }

Could you please remove the empty lines within the switch statement?

Looks good otherwise!
(Reporter)

Comment 10

a year ago
Comment on attachment 8961853 [details] [diff] [review]
(fourth attempt) 0001-Bug-1447950-use-a-switch-statement-in-place-of-chain.patch

Yep. Thanks!
Attachment #8961853 - Flags: review+
(Reporter)

Updated

a year ago
Keywords: checkin-needed

Comment 11

a year ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c616e0e41de0
use a switch statement in place of chained if-else in AsyncTabSwitcher.handleEvent. r=dao
Keywords: checkin-needed

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c616e0e41de0
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.