1431214 - Function _setCurrentURI seems to be unused

Status: RESOLVED FIXED

(Firefox :: Session Restore, enhancement, P4)

Description

[Adrian Wielgosik [:adrian17]]

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.

Comment 1

[Mike de Boer [:mikedeboer]]

I think it's safe to say that the changes can be reverted and the methods on the bindings and RemoteWebProgress.jsm removed.

Comment 2

[Matthew N. [:MattN]]

Reserving for some students on Monday.

Remove the methods added in https://hg.mozilla.org/mozilla-central/rev/5b0fdad425f8

Comment 3

[suman5214]

I would like to work on this as a good-first-bug

Comment 4

[Matthew N. [:MattN]]

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.

Comment 5

[akshayjain1996]

I am new here, and looking for a bug to work on. Can I take this on?

Comment 6

[Matthew N. [:MattN]]

(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

Comment 7

[Korina]

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.

Comment 8

[Korina]

Attachment: mypatch.diff

Comment 9

[Adrian Wielgosik [:adrian17]]

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

Comment 10

[Mike de Boer [:mikedeboer]]

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.

Comment 11

[trushita]

Hi,

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

Comment 12

[Mike de Boer [:mikedeboer]]

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

Comment 13

[:Wendy]

Attachment: Bug 1431214 - removed _setCurrentURI function as it was unused

Comment 14

[:Wendy]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e58cebce4737
removed _setCurrentURI function as it was unused r=mikedeboer

Comment 16

[Daniel Varga [:dvarga]]

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