245133 - [FIX]bad focus behaviour when moving a text input and immediately focus on it in the same function

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Mathieu Pellerin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040530 Firefox/0.8.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040530 Firefox/0.8.0+

If you call a function (say 'openMoveDiv()') that will move a text input in a
div to a new location on the page where you mouse clicked, and that you focus on
the input in the same function, the browser will scroll back to the old
div/input location.

Reproducible: Always
Steps to Reproduce:
1.Open proofofbug.html;
2.Go to the end of the page, click on 'Open div container here';
3. you can reproduce the behaviour infinitly by using the link at the beginning
of the page when your div is at the bottom, and vice-versa.

Actual Results:  
the page scrolls back to the old text input position

Expected Results:  
focus on the text using its new position

Comment 1

[Mathieu Pellerin]

Attachment: Testcase for this bug

use this file to test the bug

Comment 2

[Mathieu Pellerin]

I forgot to mention that there's an easy workaround: simply set a timeout
function (like 200ms later) that will focus the text input ... 

Comment 3

[Boris Zbarsky [:bzbarsky]]

Not at all related to the JS engine.  The JS engine knows nothing about focus,
positions, etc.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So the point is that we should be flushing reflows before trying to scroll
frames into view.  Not sure whether the flush should be in
nsPresShell::ScrollFrameIntoView or in its callers....  Putting it in
ScrollFrameIntoView makes the most sense to me -- I don't really see a
reasonable situation where we'd want to ScrollFrameIntoView _without_ flushing.

Comment 5

[Johnny Stenback (:jst)]

Agreed.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Actually, this is a little tough... The problem is that we really need to flush
out both style and layout before scrolling.  But we don't want to be flushing
out style while inside a frame impl, since the style change can tear down the frame.

So I think the right thing to do is to fix form control content impls to flush
before scrolling (like anchors and areas do).

Comment 7

[Boris Zbarsky [:bzbarsky]]

On second thought, the right fix is to make Flush_Frames flush out style
reresolves (so that any frame reconstructs get flushed) and then have
ScrollFrameIntoView flush just reflow.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

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

jst, what do you think?  Note that the GetPrimaryFrame code in
nsGenericHTMLElement flushes Flush_Frames, which is why this is happy.

Comment 10

[Johnny Stenback (:jst)]

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

Looks good. r+sr=jst

Comment 11

[Boris Zbarsky [:bzbarsky]]

Checked in.