145577 - scrollHeight/scrollTop not supported correctly on textareas

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Erik Arvidsson]

The scroll model inside html textareas is broken. The return value for
scrollHeight returns incorrectly always the same value as the height of the
textarea.

This is a severe loss of functionality because teh ability to control the size
and scrolling of textareas is maybe even more important than for other elements.

I know that the scroll model is very buggy in general but it is totally broken
for textareas. This is not acceptable.

Comment 1

[Erik Arvidsson]

Attachment: Textarea to test scrollHeight

When you write something (onkeypress) the window.status is updated with the
current scrollHeight value. The value is always 300 which is obviously
incorrect.

Comment 2

[Gérard Talbot]

I confirm that scrollHeight and scrollWidth properties do not get updated at all
(window XP, Moz1.1a+ build 20020707).

The properties clientHeight and clientWidth are also ignored on textarea
elements as they show 0.

The properties offsetHeight and offsetWidth are also quite wrong: they do not
include padding and border values on the textarea.

The properties clientLeft and clientTop are undefined for textarea (not just
specific to textarea but to all elements: see bug 62536 and 11207 on that).

Setting and/or getting scrollLeft and/or scrollTop values to a textarea also do
not work.

So, at least 6 DHTML object model properties do not work at all on textarea.
"Severe loss of functionality", "very buggy" and "totally broken" words are
reasonable and level-headed here. A complete test case will follow later (within
a few days).

Comment 3

[Gérard Talbot]

Attachment: Textarea elements mishandles all DHTML object model properties

Self explanatory meta test case for textarea elements.

Comment 4

[Gérard Talbot]

In light of what was agreed on in bug 157847, then the offsetWidth and
offsetHeight property values are correctly, accordingly computed as they include
padding and border values of the textarea.

Comment 5

[William J. Edney]

Having these properties actually work for textareas is crucial.

Comment 6

[Pete Collins (MDG)]

jst, we would get the scroll offsets for a multiline text control from nsIView
right?

--pete

Comment 7

[Garth Dahlstrom]

Is this going to be fixed?

I actually have a requirement for a page I am doing to return the user to a spot
in the textarea some ways down from the top after a submit.  

Since even moving the caret position does not change the viewable part of the
textarea (and there is no way to simulate a keypress event), the best I can do 
is create a button that says "go to last position" that restores the caret 
position and then alert the user telling them to "push an arrow key"...  

Comment 8

[Gérard Talbot]

Attachment: Textarea mishandles DHTML object model properties.

I added wrap="off" so that an horizontal scrollbar may appear in Mozilla, MSIE
6 for Windows and Opera 7. I tuned the previous testcase.

The red-bordered and yellow background colored textarea can be edited at will
to add, remove, append, delete,.. whatever content, typing in order to
display/remove scrollbar(s) at will. The affected DHTML property values will
(rather should) be updated in real time.

Comment 9

[Johnny Stenback (:jst)]

Mass-reassigning bugs to [e-mail]

Comment 10

[Harshal Pradhan]

Attachment: Possible fix ?

This appears to make things much better. Is this really as simple as this?
I dont know enough about views to be sure.

Comment 11

[Harshal Pradhan]

Comment on attachment 120974 [details] [diff] [review]
Possible fix ?

jst, jkeiser, what do you think?

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 120974 [details] [diff] [review]
Possible fix ?

   if (!scrollFrame) {
+

Loose the empty line here.

+    nsIScrollableViewProvider *scrollProvider = nsnull;
+    CallQueryInterface(frame, &scrollProvider);
+    if (scrollProvider) {
+      scrollProvider->GetScrollableView(presContext, aScrollableView);
+      return NS_OK;
+    }
+

Yeah, that looks good to me, but what if the scrollable view provider doesn't
provide a view (i.e. returns null)? IMO we should only return early here if
*aScrollableView is non-null, if not, fall through to the other code.

sr=jst with those changes.

Comment 13

[Harshal Pradhan]

To me.

Comment 14

[Harshal Pradhan]

Attachment: Fixed jst's issues

Comment 15

[Harshal Pradhan]

Comment on attachment 121113 [details] [diff] [review]
Fixed jst's issues

Marking jst's sr. John, can you review this fairly simple patch.

Comment 16

[Harshal Pradhan]

Comment on attachment 120974 [details] [diff] [review]
Possible fix ?

Ugh :-( When is bugzilla going to learn to cancel review requests on obsoleted
patches?

Comment 17

[John Keiser (jkeiser)]

Comment on attachment 121113 [details] [diff] [review]
Fixed jst's issues

oh, now I get it.  r=jkeiser.  Perhaps the body element should implement this
as well, eh ...

Comment 18

[Harshal Pradhan]

Checked in. -> Fixed.

I'm sure that there are still some problems with the scroll model, but 
hopefully things should now be only as bad as for any other element :-)

(This may fix at least a couple of other bugs, ill go through and check them 
later.)

Comment 19

[bora123]

Iframe with designMode(midas) on has the same problem. Will this also fix that 
too? This fix is not just for textarea right?

Comment 20

[bora123]

For your information, Editable IFrame still doesnt have the scrollWidth 
working. 

Comment 21

[José Jeria]

This bug was about scrollHeight in textareas, can you file a new bug about the 
midas problem?