621888 - Fennec zoom to the wrong area when the search result is at the bottom of the page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Panning/Zooming, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Because most of the zoom calculation is done using browser.getBoundingClientRect().height, zooming to an element using the form helper or the find helper could result into zooming to the wrong area.

Steps to reproduce:
 * go to http://www.google.com (non-mobile version)
 * go to the site menu and click on "Find in page"
 * in the search textbox type "back"

Actual result:
 * The UI try to zoom near the bottom of the page but fail

Expected result:
 * The UI zoom and show the "Change Background Image" link at the bottom left corner of the page.

Comment 1

[Matt Brubeck (:mbrubeck)]

Comment on attachment 500191 [details] [diff] [review]
Patch

Looks good to me.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/c126e042c0d7

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Some cleanup as asked on IRC

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 500218 [details] [diff] [review]
Patch

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js

>-  update: function va_update(aIsVKB) {
>-    Browser.styles["viewable-height"].height = (this.height - Elements.contentNavigator.getBoundingClientRect().height) + "px";
>-    Browser.styles["viewable-width"].width = this.width + "px";
>+  update: function va_update() {
>+    let oldHeight = parseInt(Browser.styles["viewable-height"].height);
>+    let newHeight = this.height - Elements.contentNavigator.getBoundingClientRect().height;
>+    if (newHeight != oldHeight) {
>+      Browser.styles["viewable-height"].height = (this.height - Elements.contentNavigator.getBoundingClientRect().height) + "px";

just use newHeight, so we can avoid anther call to getBoundingClientRect:

>+      Browser.styles["viewable-height"].height = newHeight + "px";

r+ with that change

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

followup: 
http://hg.mozilla.org/mobile-browser/rev/2cde1b090636

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Shame! I've forgot to rename a this->self...
http://hg.mozilla.org/mobile-browser/rev/6e20491a80f5

Comment 7

[Aakash Desai [:aakashd]]

verified FIXED on build:

Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20101230 Namoroka/4.0b9pre Fennec/4.0b4pre