1537535 - Use .trim() instead of a regex replace in permissions.js

Status: RESOLVED FIXED

(Firefox :: Settings UI, enhancement, P5)

Description

[Johann Hofmann [:johannh]]

This old code is using a regex to strip leading whitespace where we could just use .trim():

https://searchfox.org/mozilla-central/rev/7c20ad925005fbad7b8e08813115f1ec7fa1c248/browser/components/preferences/permissions.js#188

Technically .trimStart is the exact replacement, but it might be a good idea to remove trailing whitespace as well.

Comment 1

[afshan.shokath]

I would like to take this issue. Can I be assigned to this?

Comment 2

[Johann Hofmann [:johannh]]

Absolutely, thanks!

Comment 3

[afshan.shokath]

Thanks a lot.

Comment 4

[afshan.shokath]

Attachment: Bug-1537535 Fixed.

Changed the string replace method with trim() method

Issue fixed

Comment 5

[afshan.shokath]

Hello Johann.
So I fixed the bug and uploaded a patch to phabricator.
https://phabricator.services.mozilla.com/D24344

Please review it and let me know if it is resolved.

Comment 6

[afshan.shokath]

Attachment: Bug-1537535. In Testing phase.

I had made the required changes to the code by copying it into a new file.
Figured out that is not how I am supposed to do it.
This is my first time contributing so I am still learning.

Changed the string replace method with trim() method

Bug - 1537535.
I had added a new file instead of editing the required file.
Now I edited the required file itself.

Comment 7

[Johann Hofmann [:johannh]]

Setting checkin-needed to get this landed for us :)

Comment 8

[afshan.shokath]

Will do it ASAP. Thank you.
Also can you please point me to other good-first bugs?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c50648e40f6
Changed the string replace method with trim() method. r=johannh

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7c50648e40f6

Comment 11

[afshan.shokath]

(In reply to Johann Hofmann [:johannh] from comment #7)

Setting checkin-needed to get this landed for us :)

So the project that you are mentoring is closed for applications but I technically landed this before it was closed. I have been trying to contact you and April to discuss about project time-line but I am not receiving any reply. i know you guys are held-up neck deep and doing a great job but Can you please reply to my mail?
Thank you.

Comment 12

[April King [:April]]

I believe we are in the process of evaluating candidates and their submissions now.

Comment 13

[afshan.shokath]

(In reply to April King [:April] from comment #12)

I believe we are in the process of evaluating candidates and their submissions now.

So do you also look at submissions done after the last date? Because I was assigned (this bug) but it later on turned out to be a wont-fix. So I ended up spending almost 3 weeks on it untill Johann assigned me the new one. I also started working on one of the issues in the github repo immediately after my first submission, but I am still in process of writing a patch.

Comment 14

[Johann Hofmann [:johannh]]

We will look at and consider each application that is submitted through Outreachy before April 2nd, if the applicant has made at least one contribution according to the Outreachy rules. How we evaluate candidates is not purely based on the number of contributions.