Closed Bug 1431214 Opened 3 years ago Closed 11 months ago

Function _setCurrentURI seems to be unused

Categories

(Firefox :: Session Restore, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 73
Tracking Status
firefox73 --- fixed

People

(Reporter: adrian17, Assigned: Wendy, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(2 files)

In bug 923333, in the review, the function _setCurrentURI was introduced:

https://bugzilla.mozilla.org/attachment.cgi?id=814921&action=diff

It was used in SessionStore.jsm. However, it looks like during the push, the change to SessionStore.jsm was omitted and the new function was unused.

It should be checked whether changes in SessionStore.jsm (or ContentRestore.jsm, as it looks like some of the code was moved there) are still necessary. If not, changes from that bug can probably be reverted.
See Also: → 923333
I think it's safe to say that the changes can be reverted and the methods on the bindings and RemoteWebProgress.jsm removed.
Mentor: mdeboer
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug
Priority: -- → P4
Assignee: MattN+bmo → nobody
Status: ASSIGNED → NEW
I would like to work on this as a good-first-bug
OK, let me or Mike know if you have any questions by using the "need more information" option below the comment box. We'll assign it once you submit a patch.
I am new here, and looking for a bug to work on. Can I take this on?
(In reply to akshayjain1996 from comment #5)
> I am new here, and looking for a bug to work on. Can I take this on?

Sure, please attach a patch when you're ready. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_-_Get_your_code_reviewed
Hi, do you want the RemoteWebProgress.jsm file removed?

(In reply to Mike de Boer [:mikedeboer] from comment #1)
> I think it's safe to say that the changes can be reverted and the methods on
> the bindings and RemoteWebProgress.jsm removed.
Attached patch mypatch.diffSplinter Review
Flags: needinfo?(adrian.wielgosik)
Attachment #9013180 - Flags: review+
Attachment #9013180 - Flags: feedback+
Hi Korina, couple of suggestions:

I'm not a reviewer, just a person who reported the issue. For review and feedback, it'd be better if you tried asking mikedeboer (the person assigned as mentor of this bug). I'll needinfo? him for you. You can also ask for help on the IRC server.

The "review+" flag suggests that the patch already got a positive review, and is set by the reviewer. When submitting a patch file for review, you can set the "?" flag ("requesting review") and put the reviewer's nick in the text box.

> Hi, do you want the RemoteWebProgress.jsm file removed?

No; there are only specific changes in this file (the function in question) that are no longer needed.

(Also, your patch seems to have some changes that don't seem relevant to this issue.)
Flags: needinfo?(adrian.wielgosik) → needinfo?(mdeboer)
Comment on attachment 9013180 [details] [diff] [review]
mypatch.diff

Review of attachment 9013180 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Korina, thank you for submitting a patch!

Like Adrian mentioned, your patch contains some changes unrelated to this bug and it's missing a few code removals that were added in the following revision: https://hg.mozilla.org/mozilla-central/rev/5b0fdad425f8
The idea is to basically revert the code changes made in that revision.

Sorry for the late reply, I was on vacation for a little while. Please let me know if you have any further questions.
Attachment #9013180 - Flags: review-
Attachment #9013180 - Flags: review+
Attachment #9013180 - Flags: feedback+
Assignee: nobody → korina.houghtaling
Status: NEW → ASSIGNED
Flags: needinfo?(mdeboer)

Hi,

I understand this issue is assigned, but i would like to work on it incase no one is working.

Flags: needinfo?(mdeboer)

Sure, please feel free to work on this bug; I'll assign you to the bug once you've submitted a patch. Thanks!
You can find documentation on how to do that over at https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_Get_your_code_reviewed

Assignee: korina.houghtaling → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mdeboer)
Assignee: nobody → wiggwendy

Hey I'm new and want to start contributing. Please let me know if anything looks off!
Thanks

Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e58cebce4737
removed _setCurrentURI function as it was unused r=mikedeboer
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73
You need to log in before you can comment on or make changes to this bug.