Refactor some code in browser.js

RESOLVED FIXED in Firefox 19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
Firefox 19
All
Android
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

I have these two refactoring patches in my queue for some of the readability prototypes I was/am working on, and I would really like to land them to avoid having to keep rebasing and manually merging them.
To more easily see the diff
Comment on attachment 672325 [details] [diff] [review]
(1/2) Extract a zoomToElement function

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

::: mobile/android/chrome/content/browser.js
@@ +3773,2 @@
>  
> +  _zoomToElement: function(aElement, aClickY) {

aClickY should probably be optional. You can use the new defaultParameters stuff to make it = -1 (which we can use to force zooming to really tall elements?):

https://developer.mozilla.org/en-US/docs/JavaScript/Reference/default_parameters

@@ +3799,5 @@
> +    // if the block we're zooming to is really tall, and the user double-tapped
> +    // more than a screenful of height from the top of it, then adjust the y-coordinate
> +    // so that we center the actual point the user double-tapped upon. this prevents
> +    // flying to the top of a page when double-tapping to zoom in (bug 761721).
> +    // the 1.2 multiplier is just a little fuzz to compensate for bRect including horizontal

NIT: Update these comments to not talk about double tap and just talk about elements that are really tall.

@@ +3801,5 @@
> +    // so that we center the actual point the user double-tapped upon. this prevents
> +    // flying to the top of a page when double-tapping to zoom in (bug 761721).
> +    // the 1.2 multiplier is just a little fuzz to compensate for bRect including horizontal
> +    // margins but not vertical ones.
> +    let cssTapY = viewport.cssY + aClickY;

I think we probably want a way to skip this. I.e. wrap in a if (aClickY >= 0)
Attachment #672325 - Flags: review?(wjohnston) → review+
Attachment #672328 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/8459554cbfe9
https://hg.mozilla.org/mozilla-central/rev/02bcbfb28383
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.