The default bug view has changed. See this FAQ.

A few unused local variables in PanZoomController.onScaleEnd

RESOLVED FIXED in Firefox 11

Status

()

Firefox for Android
General
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

unspecified
Firefox 12
x86
Linux
Points:
---

Firefox Tracking Flags

(firefox11 fixed, firefox12 fixed, fennec11+)

Details

Attachments

(1 attachment)

This variables are unused and I can't tell if we wanted to use them and forgot, or if we can safely remove the code:

"o" : http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ui/PanZoomController.java#980
"viewport" : http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ui/PanZoomController.java#988
"pageRect" : http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/ui/PanZoomController.java#991
According to the blame, viewport and pageRect can be removed (accidentally left behind after other code was removed) but I'm not sure about "o" - wesj added that in his double-tap zoom support patch (bug 697701) but it was never used.
Created attachment 585411 [details] [diff] [review]
patch

Removes the unused locals. "pageSize" was only used for "pageRect" so it could be removed too.

If Wes needed "o" he can mention in feedback.
Assignee: bugmail.mozilla → mark.finkle
Attachment #585411 - Flags: review?(bugmail.mozilla)
Attachment #585411 - Flags: feedback?(wjohnston)
Attachment #585411 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 585411 [details] [diff] [review]
patch

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

I ended up doing going through more revisions of double tap zoom than i wanted. I think that was just left over cruft that snuck through :(
Attachment #585411 - Flags: feedback?(wjohnston) → feedback+
https://hg.mozilla.org/integration/mozilla-inbound/rev/91e7e04c53b5
https://hg.mozilla.org/mozilla-central/rev/91e7e04c53b5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
Comment on attachment 585411 [details] [diff] [review]
patch

[Approval Request Comment]
Code cleanup. No risk
Attachment #585411 - Flags: approval-mozilla-aurora?

Comment 7

5 years ago
Comment on attachment 585411 [details] [diff] [review]
patch

[Triage Comment]
Doesn't seem as if this is needed for Aurora since it's only removing unused variables.
Attachment #585411 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
tracking-fennec: --- → 11+
status-firefox11: --- → affected
status-firefox12: --- → fixed
(Assignee)

Updated

5 years ago
status-firefox11: affected → wontfix
Blocks: 716673
Comment on attachment 585411 [details] [diff] [review]
patch

needed due to dependencies
Attachment #585411 - Flags: approval-mozilla-aurora- → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/31ee4b6cedea
status-firefox11: wontfix → fixed
You need to log in before you can comment on or make changes to this bug.