1216307 - Remove unused method hasOtherSyncClients

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

Then, you'll need to create a patch to upload - see
https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches

To fix this bug, remove the unused method ActivityChooserModel.hasOtherSyncClients [1].

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Thanks and happy coding! ^_^

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/ActivityChooserModel.java?rev=35336ca96de2#1298

Comment 1

[mails2bibin]

I have setup the the build environment. I am able to run
./mach build
./mach package
./mach install
and run the browser in my device.

I can see "private boolean hasOtherSyncClients()" method in ActivityChooserModel.java class.
Removing the method "hasOtherSyncClients()" from the "ActivityChooserModel.java" is the fix??

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to mails2bibin from comment #1)
> Removing the method "hasOtherSyncClients()" from the
> "ActivityChooserModel.java" is the fix??

That's correct! We'll assign you to the bug once you upload a patch. :)

By the way, you can use the "Need more information from" field to give a user a persistent notification so they'll be more likely to see your comments or questions.

Comment 3

[Alex Johnson (:alex_johnson)]

Attachment: Remove unused method hasOtherSyncClients

I deleted the hasOtherSyncClients function.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b540aac68f09

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8682398 [details] [diff] [review]
Remove unused method hasOtherSyncClients

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

Looks good to me!

I made a push to our try test servers (above).

Once the push goes green, you can add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches added via checkin-needed keyword need an associated green try run. Let me know if you need help reading the results.

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e0ba952c2819

Comment 7

[Carsten Book [:Tomcat]]

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

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/4bb9371611d91c1306b65da00c949b31baa917e6
Bug 1216307 - Backout e0ba952c2819 to backout top level shareplane (bug 1140048).

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

re backout: see bug 1140048 comment 83. Since we backed out the bug 1140048 that added this bug, this does not affect any versions until bug 1140048 lands again.

Comment 10

[Alex Johnson (:alex_johnson)]

Attachment: MozReview Request: Bug 1216307 - Backed out. r?mcomella

Bug 1216307 - Backed out. r?mcomella

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8689629 [details]
MozReview Request: Bug 1216307 - Backed out. r?mcomella

https://reviewboard.mozilla.org/r/25679/#review23123

Backouts don't need reviews and I already backed it out.

Thanks for the patch though!

Comment 12

[hassani2]

I am computer science student , i would like to fix this bug , is that okay ? :)

Comment 13

[Alex Johnson (:alex_johnson)]

(In reply to hassani2 from comment #12)
> I am computer science student , i would like to fix this bug , is that okay
> ? :)

We must wait till bug 1140048 relands before we land this bug.  See comment 9.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I don't think bug 1140048 will reland soon – we need to complete bug 1217174 first, which is a bit of work and prioritized.

hassani2, would you like to talk a look at bug 1164307?

Comment 15

[:Margaret Leibovic]

Same for this bug, we should just address removing unused code in the same patch that removes the callsites.

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I'd like to leave this open so we don't forget to do it as part of bug 1140048.

Comment 17

[raunaqabhyankar]

Attachment: bug-1216307-fix.patch

Deleted method hasOtherSyncClients() as well as its callsite.

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8706468 [details] [diff] [review]
bug-1216307-fix.patch

Removing this depends on the completion of bug 1140048 first (or more likely bug 1217174).

Comment 19

[Swarnim]

Hi Michael, I am Java certified and also a newbie here.I want to resolve this bug.May be I can help you.

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Swarnim from comment #19)
> Hi Michael, I am Java certified and also a newbie here.I want to resolve
> this bug.May be I can help you.

Hi Swarnim – this bug requires bug 1140048 to be completed first. Perhaps you'd like to take a look at bug 1042201?

Comment 21

[Swarnim]

Yes Michael, I have gone through Bug 1042201 - Search provider suggestions widget for error pages.I read all the comments and attachments but still having some doubts related to that bug.Perhaps I have to talk with the Mentor:Margaret Leibovic.

Thankyou Michael

Comment 22

[Alex]

Hello,

I'am really interesting to try to fix a bug (I am beginner).
I install intelliJ,and import mozilla center, but i don't know with witch bug i can start.
Anybody can advise to me ? :)

Thank you in advance for your sugestions

Comment 23

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey Alex! Welcome to Bugzilla.

Unfortunately, this isn't a good bug to start because it requires bug 1140048 to be completed first. Perhaps you can take a look at bug 1242629?

By the way, you can find us on mozilla irc in the channel #mobile if you have further questions!

Comment 24

[Alex]

Hi Michael,
Thanks for your answer !
I will see the bug 1242629 :)

Comment 25

[Andreas Wagner [:TheOne] [use NI]]

(In reply to Michael Comella (:mcomella) from comment #23)
> Hey Alex! Welcome to Bugzilla.
> 
> Unfortunately, this isn't a good bug to start because it requires bug
> 1140048 to be completed first. Perhaps you can take a look at bug 1242629?

Wouldn't we want to remove the "good first bug" whiteboard tag then?

Comment 26

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Andreas Wagner [:TheOne] from comment #25)
> > Unfortunately, this isn't a good bug to start because it requires bug
> > 1140048 to be completed first. Perhaps you can take a look at bug 1242629?
> 
> Wouldn't we want to remove the "good first bug" whiteboard tag then?

From a correctness point of view, I don't think so: bugs that have unfixed dependencies can still be [good first bug]s (when they're ready to be worked on).

However, ideally we wouldn't broadcast [good first bug]s that have unfixed dependencies to avoid issues where someone comes in to work on a bug but doesn't see the dependencies. Unfortunately, it's too much bookkeeping to only add the [good first bug] flags once the dependencies are fixed.

I think the ideal solution here would be to update the [good first bug] tools to only display [good first bug]s that don't have dependencies.

That being said, I don't think bug 1140048 will get fixed any time soon so I'm going to remove the [good first bug] tag. Thanks for pointing it out.

Comment 27

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.