Bug 9942 – REGRESSION: Editing text on line numbers greater than 'rows' attribute makes native text area scroll up

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[David Kilzer (:ddkilzer)]

Steps to reproduce:

1. Open a web page with a text area (like this bug).
2. Enter line numbers on each line, then hit Enter, so that there are twice as many lines as the 'rows' attribute for the text area is set to.  (For Bugzilla comments, this would be 20 lines total.)
3. Move the cursor to Line 15, but make sure Line 15 is NOT the last visible line in the text area.  (Clicking to the right of the "15" only works if you're immediately next to the "5".  See Bug 9936.)
4. Type any character, like "a".

Expected results:

The text area does not scroll up.

Actual results:

The text area scrolls up so that the line being edited is at the bottom of the visible area.

Regression:

This works as expected on production Safari 2.0.4 (419.3) on Mac OS X 10.4.7 (8J135/PowerPC).

Notes:

Reproduced on a locally-built WebKit r15458 with the above versions of Safari and Mac OS X.

This may be related to Bug 9581.

[Darin Adler]

This sounds like a bug Adele is already working on.

<rdar://problem/4644614>

[Adele Peterson]

Created attachment 9617 [details]
potential fix

I don't think this is the right fix for the long term, but in the short term, this fixes the textarea case.  For example, if a div w/ overflow were nested in multiple RenderBlocks/RenderFlexibleBoxes, then this fix wouldn't apply since it only checks the direct parent.  It also wouldn't work for children that were RenderFlexibleBoxes.  It is a lower risk fix than some of the other ideas that hyatt, darin, and i discussed.

[Darin Adler]

Comment on attachment 9617 [details]
potential fix

Even if we're doing this hack, I don't think that a two-pass layout system justifies having a counter for number of layouts. How about just a boolean that's true when you're in the first pass?

[Adele Peterson]

I tried generalizing this fix a little more, and it seems that horizontal flex boxes (like in a text field) don't always cause the child to layout twice.  So the more general version fails.

[Adele Peterson]

Created attachment 9618 [details]
another patch

this doesn't change anything for horizontal boxes.

[Adele Peterson]

Created attachment 9620 [details]
patch w/ changelog & test

[Maciej Stachowiak]

<rdar://problem/4644614>

[Maciej Stachowiak]

Comment on attachment 9620 [details]
patch w/ changelog & test

I think this is good enough for right now.

[Adele Peterson]

15581

[Darin Adler]

>    if (view()->flexBoxInFirstLayout() == this) 
>        view()->setFlexBoxInFirstLayout(0); 
>    else 
>        view()->setFlexBoxInFirstLayout(this); 

Adele, the code above won't work right for nested flex boxes. You should do this instead:

>    if (view()->flexBoxInFirstLayout() == this) 
>        view()->setFlexBoxInFirstLayout(0); 
>    else if (!view()->flexBoxInFirstLayout())
>        view()->setFlexBoxInFirstLayout(this); 

which will work for the nested case too.

[Adele Peterson]

updated w/ darin's suggested change => r15582.