Closed Bug 704690 Opened 13 years ago Closed 13 years ago

html anchors do not pan to the right spot

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: nhirata, Assigned: kats)

References

()

Details

Attachments

(2 files, 2 obsolete files)

1. go to http://people.mozilla.com/~nhirata/html_tp/jumptag.html
2. click on the link "Go to Bottom"

Expected: Goes to the bottom of the page
Actual: in some cases it doesn't move.

HTC Flyer; 20111122; Android 2.3
related to Bug 703620 ?
Assignee: nobody → kgupta
Attachment #578056 - Flags: review?(chrislord.net)
In the test URL on this bug, I often end up flinging/snapping the page when clicking on the "bottom" link to go back to the top. This was causing the viewport to get clobbered by the FlingRunnable, so I needed this patch to make it work.
Attachment #578057 - Flags: review?(chrislord.net)
Comment on attachment 578056 [details] [diff] [review]
(1/2) Force viewport update on anchor jump

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

r+, but would prefer to see the comment below addressed.

::: mobile/android/chrome/content/browser.js
@@ +1185,5 @@
> +        gecko: {
> +          type: "Viewport:Update",
> +          viewport: JSON.stringify(this.viewport)
> +        }
> +      });

Why doesn't this use sendViewportUpdate? I would suggest refactoring sendViewportUpdate so it can be used for things like this, so we don't have these last few lines in multiple places.
Attachment #578056 - Flags: review?(chrislord.net) → review+
Comment on attachment 578057 [details] [diff] [review]
(2/2) Abort flings on viewport update

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

r+, with the comment addressed (either via a rationalisation or a code change, either is fine by me)

::: mobile/android/base/ui/PanZoomController.java
@@ +133,5 @@
>      public void geometryChanged() {
>          populatePositionAndLength();
>      }
>  
> +    public void viewportMetricsChanged() {

Can we just have a boolean flag in geometryChanged, something like aCancelTouch or aReset (or some other better name that escapes me)? Having a separate function for what is essentially the same event, but from different sources could become confusing down the line.
Attachment #578057 - Flags: review?(chrislord.net) → review+
Updated with suggested refactoring.
Attachment #578056 - Attachment is obsolete: true
Attachment #578243 - Flags: review+
(In reply to Chris Lord [:cwiiis] from comment #5)> 
> Can we just have a boolean flag in geometryChanged, something like
> aCancelTouch or aReset (or some other better name that escapes me)? Having a
> separate function for what is essentially the same event, but from different
> sources could become confusing down the line.

Yeah, that makes more sense, fixed.
Attachment #578057 - Attachment is obsolete: true
Attachment #578244 - Flags: review+
Samsung Galaxy SII (Android 2.3.4)
20111202040206
http://hg.mozilla.org/projects/birch/rev/e2a54aafac18
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.