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

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P5
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: johannh, Assigned: afshan.shokath, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 68
Points:
---

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

47 bytes, text/x-phabricator-request
Details | Review

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.

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

Absolutely, thanks!

Assignee: nobody → afshan.shokath
Status: NEW → ASSIGNED

Thanks a lot.

Posted file Bug-1537535 Fixed.

Changed the string replace method with trim() method

Issue fixed

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.

Posted file Bug-1537535. In Testing phase. (obsolete) —

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.

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

Keywords: checkin-needed

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

Attachment #9052663 - Attachment is obsolete: true

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c50648e40f6
Changed the string replace method with trim() method. r=johannh

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

(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.

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

(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.

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.

You need to log in before you can comment on or make changes to this bug.