Closed Bug 1486313 Opened 2 years ago Closed 2 years ago

Replace makeURI with Services.io.newURI in pageInfo.js

Categories

(Firefox :: Page Info Window, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: johannh, Assigned: arshadkazmi42, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We should replace all occurrences of the deprecated makeURI function with Services.io.newURI here:

https://searchfox.org/mozilla-central/search?q=makeURI&case=false&regexp=false&path=pageInfo.js

The parameters should stay the same.
I want to take this up
Flags: needinfo?(jhofmann)
Awesome, let me know if you need any help! Thanks!
Assignee: nobody → arshadkazmi42
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
It needs to be changed only in that file? Or I need to check other files also for this change?
Flags: needinfo?(jhofmann)
Done with the changes.

Added to phabricator and added you as reviewer

https://phabricator.services.mozilla.com/D4389
Hey Johann,
Any updates on this? I am not sure who is going to do review for this. I have added you as reviewer for this.
Comment on attachment 9004362 [details]
Bug 1486313 makeURI replaced with Services.io.newURI

Johann Hofmann [:johannh] has approved the revision.
Attachment #9004362 - Flags: review+
(In reply to Arshad Kazmi from comment #6)
> Hey Johann,
> Any updates on this? I am not sure who is going to do review for this. I
> have added you as reviewer for this.

Hey Arshad, sorry for the delay, the last days were a bit busy for me.

Thank you for the reminder, that patch looks great!

You now have editbugs access (which means you can edit all bugs), would you like to try it out and set the checkin-needed flag yourself? :)

Thanks!
Flags: needinfo?(jhofmann)
Flags: needinfo?(jhofmann)
Attachment #9004362 - Flags: checkin?(jhofmann)
I have added 'checkin flag'

Is this the right way?
Adding '?'
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5deec145c04d
makeURI replaced with Services.io.newURI r=johannh
Hey Johann,
I think code is merged now. 
What is the next step for this bug? Do I have to close it? or there is some flag for it?
arshadkazmi42, can you please remove the checkin? because this patch is still in the list on checkin-needed bugs. 
Next time you can just add the 'checkin-needed' in the keywords section. Thanks!
Flags: needinfo?(arshadkazmi42)
Flags: needinfo?(arshadkazmi42)
Attachment #9004362 - Flags: checkin?(jhofmann)
Hi Eliza,
Updated the flag. I got it now from my other bug flow, that checkin-needed needs to be added in the keywords.

So can I add checkin-needed keyword in this bug now? Or its already checkedin?
Flags: needinfo?(ebalazs)
This patch is already landed. Please see Comment 10.
Flags: needinfo?(ebalazs)
Oh. yes. missed that comment. 
So what is the next step? This bug needs to be closed?
Do I have to close the bug? Or there is someone else who handles this?
Flags: needinfo?(ebalazs)
https://hg.mozilla.org/mozilla-central/rev/5deec145c04d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
No need to take any other action, as you can see in the previous comment your patch was merged in mozilla-central and it was closed as Resolved fixed.
Flags: needinfo?(ebalazs)
Ok. thank your for the details :)
Flags: needinfo?(jhofmann)
You need to log in before you can comment on or make changes to this bug.