705106 - Page should pan as you type if the text field goes beyond the visible area

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Cristian Nicolae (:xti)]

Mozilla/5.0 (Android;Linux armv7l;rv:11.0a1)Gecko/20111123
Firefox/11.0a1 Fennec/11.0a1
Devices: Samsung Galaxy Nexus S
OS: Android 2.3.4

Steps to reproduce:
1. Open Fennec App
2. Browse to bugzilla.mozilla.org
3. Perform a zoom-in and pan the page to the right until the search field is half-visible on the screen
4. Type at least 20-25 characters

Expected result:
After step 4, the page should pan to the right as you type into the text field until it is fully visible. The page should not pan further.

Actual result:
The page doesn't pan at all, even if the cursor reaches and goes by the visible side of the text field.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

The fundamental problem here seems to be that ScrollToShowRect in nsPresShell.cpp uses aScrollFrame->GetScrollPortRect() which doesn't take into account the scale at which the compositor is painting. Still working on confirming and that figuring out what the correct fix here is.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

This seems to do the job.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

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

::: layout/base/nsPresShell.cpp
@@ +3097,5 @@
> +  nsSize visibleSize = aScrollFrame->GetScrollPortRect().Size();
> +  if (aPresShell->IsScrollPositionClampingScrollPortSizeSet()) {
> +    visibleSize = aPresShell->GetScrollPositionClampingScrollPortSize();
> +  }
> +  nsRect visibleRect(scrollPt, visibleSize);

I think we should add a method to nsIScrollableFrame called GetScrollPositionClampingRange or something like that, and that method should be responsible for doing this extra work, and we should just call that method here.

Also, you'll need to check whether the scrollframe is for the root. Currently you're overriding for every scrollframe in the presshell, which is definitely wrong.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch (v2)

This one refactors the existing code nsGfxScrollFrameInner::GetScrollRangeForClamping and extracts a method to return the correct scroll port size. That method is exposed via nsIScrollableFrame and used in ScrollToShowRect.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12ec56310403

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/12ec56310403

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 631432 [details] [diff] [review]
Patch (v2)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: when typing in content textfields, the caret could go out of view. also if you are scrolled far away from the textfield that has focus, typing might make the page appear to jump randomly
Testing completed (on m-c, etc.): on m-c for a day so far. needs more bake time (a couple more days at least), but nom'ing now to get it on the radar.
Risk to taking this patch (and alternatives if risky): should affect mobile only, since we're the only ones that set a ScrollPositionClampingScrollPortSize. slight risk that other pieces of code that automatically scroll something into view start scrolling to the wrong area.
String or UUID changes made by this patch: none

Nom'ing for aurora only since it's probably too risky for beta as well.

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 631432 [details] [diff] [review]
Patch (v2)

[Triage Comment]
Good fix, and affects mobile only. Should be obvious that we need to back this out if there are any regressions.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/558bfc7bcb89

Comment 12

[Adrian Tamas (:AdrianT)]

Text fields now scroll as you type.

Verified on:
Nightly 16.0a1 2012-07-09/Aurora 15.0a2 2012-07-09
HTC Desire
Android 2.2.2