Make sure the text field is visible before focus #1679

Merged
merged 3 commits into from Feb 19, 2013

6 participants

@mrcarlberg
Cappuccino member

so the browser will not scroll without the NSScrollView knowing about it.
Issue #1675 and maybe a little of issue #1301.
This fix will not work without pull
request #1678 - Fixed scrollRectToVisible in CPView.

The problem can be tested here (from issue #1675)

The same application with the fix and pull request #1678 can be tested here

@mrcarlberg mrcarlberg Make sure the text field is visible before focus so the browser
will not scroll without the NSScrollView knowing about it
Issue #1675 and maybe a little of issue #1301
This fix will not work without pull
request #1678 - Fixed scrollRectToVisible in CPView
182f4c3
@cappbot

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

@aljungberg
Cappuccino member

Shouldn't it, in theory, be possible to focus a text field that's not on screen? It's certainly possible in Cocoa, and it's up to the developer to add any automatic scrolling if they want it. (To test this in Cocoa, create a scroll view with two text fields and scroll until you can only see one of them. Then select it, tab to the other one and start typing. Your scroll position doesn't change, but if you later scroll the textfield into view the result of your typing is visible.)

Perhaps what we need is to figure out how to prevent the browser from messing with our scroll position in the first place.

@tolmasky
Cappuccino member

It should, but if I recall correctly it isn't possible when dealing with scrolling divs. We do it all the time though by placing textfields at negative world coordinates, but if the browser can scroll to the textfield, it will. I think mrcarlberg's approach may be the best one here.

@mrcarlberg
Cappuccino member

True, we can't take control of scrolling divs. My approach may not be what we want, but it will at least work. Today it is broken as you can see in the above example.

@aljungberg
Cappuccino member
@cappbot

Milestone: 0.9.6. Labels: #accepted, AppKit, bug. What's next? A reviewer should examine this issue.

@cappbot

Milestone: 0.9.7. Labels: #accepted, AppKit, bug. What's next? A reviewer should examine this issue.

@ahankinson

I can't get the fix pages to load, but I can verify that the code merges cleanly with master, and with #1678 applied builds and tests fine.

+#ready-to-commit

@cappbot

Milestone: 0.9.7. Labels: #accepted, #ready-to-commit, AppKit, bug. What's next? The changes for this issue are ready to be committed by a member of the core team.

@aparajita

@aljungberg If you are good with this, please merge it.

@ahankinson

Also possibly related to #1674.

@aparajita aparajita merged commit ceeaab7 into cappuccino:master Feb 19, 2013
@aparajita

It looks okay, let's see what happens

#fixed

@cappbot

Milestone: 0.9.7. Labels: #fixed, AppKit, bug. What's next? This issue is considered successfully resolved.

@mrcarlberg mrcarlberg deleted the mrcarlberg:scroll_before_focus_text_field branch Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment