GeckoPreferences.PREFS_ZOOMED_VIEW_ENABLED is unused

RESOLVED FIXED in Firefox 46

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcomella, Assigned: Samuel Mendes, Mentored)

Tracking

unspecified
Firefox 46
All
Android
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

2 years ago
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 declaration & definition of GeckoPreferences.PREFS_ZOOMED_VIEW_ENABLED [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]: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferences.java?rev=6a3a062f53cb#121
(Assignee)

Comment 1

2 years ago
I'd like to take this one as my first bug. However, I need to set up a build environment first.

Should I wait until I set it?
Flags: needinfo?(michael.l.comella)
(Reporter)

Comment 2

2 years ago
Hey Samuel – welcome to Bugzilla! :) For first bugs, we wait until there's a patch submitted before assigning the bug. As such you'll need to set up your build environment, create a patch, and submit it. If you need help at all during this process, feel free to ask questions in this bug or on irc.
Flags: needinfo?(michael.l.comella)
(Assignee)

Comment 3

2 years ago
Hi Michael, thank you for your reply. 

I'll try to set up my environment asap to come back here.
(Assignee)

Comment 4

2 years ago
Created attachment 8696253 [details] [diff] [review]
Removing the declaration and reference of GeckoPreferences.PREFS_ZOOMED_VIEW_ENABLED
Attachment #8696253 - Flags: review?(michael.l.comella)
(Assignee)

Updated

2 years ago
Attachment #8696253 - Attachment description: Removing the declaration and reference of unused the declaration & definition of GeckoPreferences.PREFS_ZOOMED_VIEW_ENABLED → Removing the declaration and reference of GeckoPreferences.PREFS_ZOOMED_VIEW_ENABLED
(Assignee)

Comment 5

2 years ago
Created attachment 8696258 [details] [diff] [review]
bug_1228037_remove_geckoPreferences.patch
Attachment #8696253 - Attachment is obsolete: true
Attachment #8696253 - Flags: review?(michael.l.comella)
Attachment #8696258 - Flags: review?(michael.l.comella)
(Reporter)

Comment 6

2 years ago
Comment on attachment 8696258 [details] [diff] [review]
bug_1228037_remove_geckoPreferences.patch

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

Hey Samuel. The patch looks good to me but it doesn't apply on the latest fx-team so I can't push it to our try test servers. Do you mind rebasing your patch? Our source files have moved so that's likely the issue. You can find them at mobile/android/base/java/...
Attachment #8696258 - Flags: review?(michael.l.comella) → review+
(Reporter)

Comment 7

2 years ago
Once the patch is rebased, we can send it to our test servers and once the push goes green, we can land it.

Also, I apologize for the delay – the team is at a conference this week so I've been intermittently available. I'll do my best to get to future patches more quickly.
Assignee: nobody → samuelmendes.dev
(Reporter)

Comment 8

2 years ago
Please use the "Need more information from" field when you post the rebased patch so I know to look at it.
(Assignee)

Comment 9

2 years ago
Created attachment 8697845 [details] [diff] [review]
bug_1228037_remove_geckoPreferences.patch
Attachment #8696258 - Attachment is obsolete: true
Flags: needinfo?(michael.l.comella)
Attachment #8697845 - Flags: review?(michael.l.comella)
(Reporter)

Comment 10

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5897b8781c22
(Reporter)

Comment 11

2 years ago
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
Flags: needinfo?(michael.l.comella)
(Reporter)

Updated

2 years ago
Attachment #8697845 - Flags: review?(michael.l.comella) → review+
(Assignee)

Comment 12

2 years ago
There was a Robocop failed test but after while, it passed. 

Seems to me that everything went ok, right?
Flags: needinfo?(michael.l.comella)
(Reporter)

Comment 13

2 years ago
Our test suites often get intermittent failures and this looks like one of them. Our harness will automatically rerun failures (as you can see from the green push). Additionally, if you click a failing test suite, you can look at the bar on the bottom of the screen and see if it matches any existing known intermittent failures, which are also listed on that bar (your failure does match).

This looks good to me and is ready for check-in!
Flags: needinfo?(michael.l.comella) → needinfo?(samuelmendes.dev)
(Assignee)

Updated

2 years ago
Flags: needinfo?(samuelmendes.dev)
Keywords: checkin-needed

Comment 14

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/ea9df316f1cc
Keywords: checkin-needed

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ea9df316f1cc
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(Reporter)

Comment 16

2 years ago
Hey Samuel! If you'd like to keep helping out, may I suggest bug 1166833 or bug 1091826?
You need to log in before you can comment on or make changes to this bug.