Closed
Bug 1330009
Opened 8 years ago
Closed 8 years ago
Remove BrowserToolbar.setToolBarButtonsAlpha()
Categories
(Firefox for Android Graveyard :: General, defect, P5)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
Firefox 53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: sebastian, Assigned: franciscomaguiar, Mentored)
Details
(Whiteboard: [good first bug][lang=java])
Attachments
(2 files, 1 obsolete file)
BrowserToolbar.setToolBarButtonsAlpha() is unused and can be removed:
https://dxr.mozilla.org/mozilla-central/rev/7011ed1427de2b6f075c46cc6f4618d3e9fcd2a4/mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java#658-663
--
To start, set up a build environment - you can see the instructions here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build
To fix this bug, you need to remove the obsolete method:
https://dxr.mozilla.org/mozilla-central/rev/7011ed1427de2b6f075c46cc6f4618d3e9fcd2a4/mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java#658-663
Then, you'll need to upload a patch - see:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html
If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me and other helpful folks in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC
Comment 1•8 years ago
|
||
Hi Sebastian
I am new to open source and would like to contribute.
I feel this can be a good first bug for me and hope you assign it to me.
I am currently in the process of setting up the environment.(In reply to Sebastian Kaspari (:sebastian) from comment #0)
> BrowserToolbar.setToolBarButtonsAlpha() is unused and can be removed:
>
> https://dxr.mozilla.org/mozilla-central/rev/
> 7011ed1427de2b6f075c46cc6f4618d3e9fcd2a4/mobile/android/base/java/org/
> mozilla/gecko/toolbar/BrowserToolbar.java#658-663
>
> --
>
> To start, set up a build environment - you can see the instructions here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/
> Build_Instructions/Simple_Firefox_for_Android_build
>
> To fix this bug, you need to remove the obsolete method:
> https://dxr.mozilla.org/mozilla-central/rev/
> 7011ed1427de2b6f075c46cc6f4618d3e9fcd2a4/mobile/android/base/java/org/
> mozilla/gecko/toolbar/BrowserToolbar.java#658-663
>
> Then, you'll need to upload a patch - see:
> http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/
> commits.html
>
> If you need any help, you can reply to this bug, or feel free to message me
> on IRC - my nick is "sebastian" and you can find me and other helpful folks
> in #mobile. If you need IRC setup instructions, see
> https://wiki.mozilla.org/IRC
Reporter | ||
Comment 2•8 years ago
|
||
Hi and welcome, Pankaj!
You can go ahead and write a patch for this bug. Once you upload a first patch we assign the bug to you. :)
Assignee | ||
Comment 3•8 years ago
|
||
I'm having a problem setting up my computer, in the mach package command i get the following message:
No command 'mach' found, did you mean:
Command 'match' from package 'mailavenger' (universe)
mach: command not found
and i have my android phone connected to my pc in debugging mode
Reporter | ||
Comment 4•8 years ago
|
||
It should be in the root folder of your checkout of mozilla-central. So if you are inside that folder then just:
> ./mach build
Assignee | ||
Comment 5•8 years ago
|
||
im in the following directory:
maegner@maegner-X550JX:~/Documents/OpenSource/Mozilla$ ls
Bootstrap.py mozilla-central
and i run ./mach build and i get:
bash: ./mach: No such file or directory
Reporter | ||
Comment 6•8 years ago
|
||
Try inside the mozilla-central folder :)
Assignee | ||
Comment 7•8 years ago
|
||
yeah it worked, thx! currently building with de mach build command and when it's done i'll get to fixing the bug!
Assignee | ||
Comment 8•8 years ago
|
||
I've finished setting up and removed the method, now how do i make sure everything is okay with the app and finaly make my commit?
Assignee | ||
Comment 9•8 years ago
|
||
Had a problem creating the message for the patch and didn't know how to change it afterwards.
I just deleted the function BrowserToolbar.setToolBarButtonsAlpha() previously located between the lines 658 to 663 from the file BrowserToolbar.java in the directory:
mozilla-central/mobile/android/base/java/org/mozilla/gecko/toolbar
Attachment #8825981 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 10•8 years ago
|
||
If you are using mercurial then you can edit the last commit (adding changes or updating the message) using:
> hg commit --amend
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → franciscommaguiar
Status: NEW → ASSIGNED
Assignee | ||
Comment 11•8 years ago
|
||
thank you :sebastian how that i've been assigned what am i supposed to do?
Reporter | ||
Comment 12•8 years ago
|
||
It would be helpful if you could update the patch with a commit message and upload the new patch. In the meantime I'll review it and push it to our 'try' servers that will run the tests and see if this causes any problems.
For future patches it's super helpful to not upload a patch file but instead push the file to mozreview:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html
Assignee | ||
Comment 13•8 years ago
|
||
I've added the message but i'm having a problem making the push;
When i run the command hg push review
I get the following message:
pushing to https://reviewboard-hg.mozilla.org/autoreview
searching for appropriate review repository
redirecting push to https://reviewboard-hg.mozilla.org/gecko
running checkstyle... failed (e.g. Java was not installed) - ignoring.
To see full error output, run locally with: `./mach gradle checkstyle`
(adding commit id to 1 changesets)
warning: Watchman unavailable: A non-recoverable condition has triggered. Watchman needs your help!
The triggering condition was at timestamp=1484236701: inotify-add-watch(/home/maegner/Documents/OpenSource/Mozilla/mozilla-central/mobile/android/tests/browser/robocop/src) -> The user limit on the total number of inotify watches was reached; increase the fs.inotify.max_user_watches sysctl
All requests will continue to fail with this message until you resolve
the underlying problem. You will find more information on fixing this at
https://facebook.github.io/watchman/docs/troubleshooting.html#poison-inotify-add-watch
abort: uncommitted changes
Reporter | ||
Comment 14•8 years ago
|
||
Mh, in this case maybe upload a patch file for now.
(In reply to Francisco Aguiar from comment #13)
> To see full error output, run locally with: `./mach gradle checkstyle`
Did you try running this command? Did it output any errors?
Assignee | ||
Comment 15•8 years ago
|
||
I got something like:
It looks like you tried to run a mach command from an invalid context. The gradle
command failed to meet the following conditions:
is_android - Must have an Android build.
Run |mach help| to show a list of all commands available to the current context.
Assignee | ||
Comment 16•8 years ago
|
||
Attachment #8825981 -
Attachment is obsolete: true
Attachment #8825981 -
Flags: review?(s.kaspari)
Attachment #8826274 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 17•8 years ago
|
||
(In reply to Francisco Aguiar from comment #15)
> I got something like:
> It looks like you tried to run a mach command from an invalid context. The
> gradle
> command failed to meet the following conditions:
>
> is_android - Must have an Android build.
>
> Run |mach help| to show a list of all commands available to the current
> context.
This is weird - did you actually go through the whole setup process and build the app?
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build
Reporter | ||
Comment 18•8 years ago
|
||
Comment on attachment 8826274 [details] [diff] [review]
Now with message explaining the fix.
Review of attachment 8826274 [details] [diff] [review]:
-----------------------------------------------------------------
The local build failed. It seems like this method needs to be removed from at least BrowserToolbarTablet too.
Please make sure you have setup the build correctly and can still build and run the app before uploading a new patch.
Attachment #8826274 -
Flags: review?(s.kaspari) → review-
Assignee | ||
Comment 19•8 years ago
|
||
I will look it up and upload a new patch ASAP.
About the process to build the app i did but i had some problems with rust and some other software in the mean time so it might have corrupted the installation i'll make sure to re-build everything again to see where the push is having problems.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8826994 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 21•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f18ac5367e0a6f482299f829b85eca22a91b33c
Bug 1330009 - Remove unused setToolBarButtonsAlpha() method. r=sebastian
Reporter | ||
Comment 22•8 years ago
|
||
mozreview-review |
Comment on attachment 8826994 [details]
Bug1330009
https://reviewboard.mozilla.org/r/104832/#review105964
Thank you! This is looking good and I already landed the patch. :)
Attachment #8826994 -
Flags: review?(s.kaspari) → review+
Assignee | ||
Comment 23•8 years ago
|
||
Thank you for helping me through the process. Now do I have to do anything else in regards to this bug?
Reporter | ||
Comment 24•8 years ago
|
||
Everything is done here. The bug will be closed as soon as the code is merged to mozilla-central. :)
Assignee | ||
Comment 25•8 years ago
|
||
So should i set the status to Resolved?
Reporter | ||
Comment 26•8 years ago
|
||
No, this will happen automatically as soon as the code is merged.
Comment 27•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•