Last Comment Bug 714711 - A few unused local variables in PanZoomController.onScaleEnd
: A few unused local variables in PanZoomController.onScaleEnd
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86 Linux
: P3 normal (vote)
: Firefox 12
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
: Sebastian Kaspari (:sebastian)
Mentors:
Depends on:
Blocks: 716673
  Show dependency treegraph
 
Reported: 2012-01-02 21:02 PST by Mark Finkle (:mfinkle) (use needinfo?)
Modified: 2012-01-16 22:38 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
patch (1.31 KB, patch)
2012-01-03 07:55 PST, Mark Finkle (:mfinkle) (use needinfo?)
bugmail: review+
wjohnston2000: feedback+
blassey.bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Mark Finkle (:mfinkle) (use needinfo?) 2012-01-02 21:02:39 PST
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
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-03 07:00:23 PST
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.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-03 07:55:40 PST
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.
Comment 3 Wesley Johnston (:wesj) 2012-01-03 09:08:27 PST
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 :(
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-03 09:23:09 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/91e7e04c53b5
Comment 5 Marco Bonardo [::mak] 2012-01-04 04:44:20 PST
https://hg.mozilla.org/mozilla-central/rev/91e7e04c53b5
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-06 06:25:17 PST
Comment on attachment 585411 [details] [diff] [review]
patch

[Approval Request Comment]
Code cleanup. No risk
Comment 7 Alex Keybl [:akeybl] 2012-01-06 11:09:28 PST
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.
Comment 8 Brad Lassey [:blassey] (use needinfo?) 2012-01-12 11:51:44 PST
Comment on attachment 585411 [details] [diff] [review]
patch

needed due to dependencies
Comment 9 Mark Finkle (:mfinkle) (use needinfo?) 2012-01-16 22:38:27 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/31ee4b6cedea

Note You need to log in before you can comment on or make changes to this bug.