1478855 - Use ES6 default parameter in updateBrowserRemotenessByURL

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, enhancement, P3)

Description

[Dão Gottwald [:dao]]

updateBrowserRemotenessByURL is currently written like this:

  updateBrowserRemotenessByURL(aBrowser, aURL, aOptions) {
    aOptions = aOptions || {};
    ...

https://searchfox.org/mozilla-central/rev/d47c829065767b6f36d29303d650bffb7c4f94a3/browser/base/content/tabbrowser.js#1675-1676

We should use an ES6 default parameter instead:

  updateBrowserRemotenessByURL(aBrowser, aURL, aOptions = {}) {
    ...

Comment 1

[Niranjana]

I would like to work on this as this is gng to be my first contribution.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Niranjana from comment #1)
> I would like to work on this as this is gng to be my first contribution.

Do you have the source code? Have you read instructions on how to create a patch?

Comment 3

[Niranjana]

I don't have source code. Can you give me the link that I have to follow to pick up the source code.

Comment 4

[jwilander]

Hi! I'd like to work on this as my first contribution. I have the source code. I'm just wondering, is the bug legit to just change the code from aOptions to aOptions = {} and remove the line below? There must be something more to it. I'm new to this haha

Comment 5

[Dão Gottwald [:dao]]

(In reply to Niranjana from comment #3)
> I don't have source code. Can you give me the link that I have to follow to
> pick up the source code.

See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code/Mercurial

(In reply to jwilander from comment #4)
> Hi! I'd like to work on this as my first contribution. I have the source
> code. I'm just wondering, is the bug legit to just change the code from
> aOptions to aOptions = {} and remove the line below? There must be something
> more to it. I'm new to this haha

No, that's indeed all. :) Good first bugs tend to be simple because their point is to give newcomers a chance to get familiar with the development process.

Comment 6

[jwilander]

Alright! Now just need to figure out how to create a patch :)

Comment 7

[Sahil Bhosale]

Can I work on this issue??

Comment 8

[Dão Gottwald [:dao]]

(In reply to Sahil Bhosale from comment #7)
> Can I work on this issue??

Yeah... Looks like jwilander gave up. :/

Comment 9

[Sahil Bhosale]

Attachment: mybug.patch

Bug 1478855
Use ES6 default parameter in updateBrowserRemotenessByURL

Made the changes.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 9000408 [details] [diff] [review]
mybug.patch

>-  updateBrowserRemotenessByURL(aBrowser, aURL, aOptions) {
>-    aOptions = aOptions || {};
>+  updateBrowserRemotenessByURL(aBrowser, aURL, aOptions = {}) {
> 
>     if (!gMultiProcessBrowser)
>       return this.updateBrowserRemoteness(aBrowser, false);

Can you please remove the empty line?

Comment 11

[Sahil Bhosale]

before the if statement??

Comment 12

[Sahil Bhosale]

Attachment: mybug2.patch

Made the required change.

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 9000487 [details] [diff] [review]
mybug2.patch

This looks like it applies after the previous patch. Instead we'll need all changes in one patch.

Comment 14

[Sahil Bhosale]

Attachment: bug.patch

Done.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 9000736 [details] [diff] [review]
bug.patch

Looks good, thanks!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed3e05886ae8
Use ES6 default parameter in updateBrowserRemotenessByURL. r=dao

Comment 17

[Eliza Balazs [:ebalazs_]]

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