829606 - input text form element "resets" view when value is changed

Status: RESOLVED FIXED

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

Description

[Nick Libreman]

Attachment: value change test

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:19.0) Gecko/20130107 Firefox/19.0
Build ID: 20130107082244

Steps to reproduce:

I change the value of an INPUT text element to remove diacritics.

Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:19.0) Gecko/20130107 Firefox/19.0


Actual results:

When the text is longer than the size of the input element, the view of the element gets reset to the start every time the value is changed through JS so the user can not see what he's typing (the caret position is ok but the view is not following it)


Expected results:

The view should follow the caret so when the user is typing and the text is longer than the size of the element, he should see the end of the text as he's typing it ... not the start. This works as expected in Chromium - see for reference (it gets reset only after it loses focus).

It seems the same problem was reported 9 years ago but there is nobody watching that one it seems (Bug 230474). A very basic syntax to test this is attached.

Comment 1

[Manuela Muntean [Away]]

I was able to reproduce this issue on the latest Nightly.

User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130114 Firefox/21.0
Build ID: 20130114031033

Comment 2

[(no longer active)]

Is this a regression?

Comment 3

[Mounir Lamouri (:mounir)]

FWIW, Firefox 17 is affected.

Comment 4

[Manuela Muntean [Away]]

(In reply to :Ehsan Akhgari from comment #2)
> Is this a regression?

I get the same faulty behaviour on Firefox 4, so it's not a regression.

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build ID: 20110318052756

Comment 5

[Mounir Lamouri (:mounir)]

What about Firefox 3.6? IIRC, there was a huge refactoring regarding the editor ownership that landed for Firefox 4.0. Maybe this is a regression from that?

Comment 6

[Nick Libreman]

It seems this goes way back to 1.0 according to Bug 230474

Comment 7

[Manuela Muntean [Away]]

(In reply to Mounir Lamouri (:mounir) from comment #5)
> What about Firefox 3.6? IIRC, there was a huge refactoring regarding the
> editor ownership that landed for Firefox 4.0. Maybe this is a regression
> from that?

It's also reproducible on Firefox 3.6.

Comment 8

[Igilama]

Hi, is anything going to happen with this bug? Is there any chance somebody might work on it? Thanks.

Comment 9

[Mounir Lamouri (:mounir)]

Attachment: Patch

Comment 10

[Igilama]

Thanks! It's super awesome that you responded so fast, I hope the patch will be accepted ;)

Comment 11

[Boris Zbarsky [:bzbarsky]]

Mounir, have you checked why that code was there?  Why is it ok to just remove it completely?

Comment 12

[Mounir Lamouri (:mounir)]

(In reply to Boris Zbarsky (:bz) from comment #11)
> Mounir, have you checked why that code was there?  Why is it ok to just
> remove it completely?

The blame goes back to hg@. Bonsai gives a refactor work back from 2002 (bug 129909). I don't know how to use bonsai to look at the file that was refactored to nsTextControlFrame.cpp to know if there is any information about this code there and I doubt there will be any information...

My feeling is that that code was there to have a better UX when a text field was changed from a long value to another long value (using .value) so the updated value is shown from the beginning. However, this might be true in that specific case but makes things not very good in most cases (like if append something to a long string).

Comment 13

[Boris Zbarsky [:bzbarsky]]

> I don't know how to use bonsai to look at the file that was refactored to
> nsTextControlFrame.cpp

You look in the Attic.  Specifically, http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla%2Flayout%2Fhtml%2Fforms%2Fsrc%2FAttic%2FnsGfxTextControlFrame2.cpp

Specifically, that was fixing bug 49339, where there are steps to reproduce for single-line textfields and whatnot.  And the code it was changing was added to fix bug 36130.  That was dealing with a similar problem: the content shrinking from a long value to a short value and suddenly nothing being visible at all in the text input.  _That_ is what we should make sure to not reintroduce.

Comment 14

[Mounir Lamouri (:mounir)]

(In reply to Boris Zbarsky (:bz) from comment #13)
> > I don't know how to use bonsai to look at the file that was refactored to
> > nsTextControlFrame.cpp
> 
> You look in the Attic.  Specifically,
> http://bonsai.mozilla.org/cvsblame.
> cgi?file=mozilla%2Flayout%2Fhtml%2Fforms%2Fsrc%2FAttic%2FnsGfxTextControlFram
> e2.cpp
> 
> Specifically, that was fixing bug 49339, where there are steps to reproduce
> for single-line textfields and whatnot.  And the code it was changing was
> added to fix bug 36130.  That was dealing with a similar problem: the
> content shrinking from a long value to a short value and suddenly nothing
> being visible at all in the text input.  _That_ is what we should make sure
> to not reintroduce.

I actually tested that case when I wrote the patch and it was working great.

Using the previous test cases, this one
 https://bugzilla.mozilla.org/attachment.cgi?id=7677
behaves the same with and without the patch.

And this one
 https://bugzilla.mozilla.org/attachment.cgi?id=10216
no longer scrolls to the top which I believe is a okay behaviour.

I should probably write an automated test for that I guess.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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

Thanks for checking that.

r=me

Comment 16

[Mounir Lamouri (:mounir)]

(In reply to Mounir Lamouri (:mounir) from comment #14)
> (In reply to Boris Zbarsky (:bz) from comment #13)
> > > I don't know how to use bonsai to look at the file that was refactored to
> > > nsTextControlFrame.cpp
> > 
> > You look in the Attic.  Specifically,
> > http://bonsai.mozilla.org/cvsblame.
> > cgi?file=mozilla%2Flayout%2Fhtml%2Fforms%2Fsrc%2FAttic%2FnsGfxTextControlFram
> > e2.cpp
> > 
> > Specifically, that was fixing bug 49339, where there are steps to reproduce
> > for single-line textfields and whatnot.  And the code it was changing was
> > added to fix bug 36130.  That was dealing with a similar problem: the
> > content shrinking from a long value to a short value and suddenly nothing
> > being visible at all in the text input.  _That_ is what we should make sure
> > to not reintroduce.
> 
> I actually tested that case when I wrote the patch and it was working great.
> 
> Using the previous test cases, this one
>  https://bugzilla.mozilla.org/attachment.cgi?id=7677
> behaves the same with and without the patch.

I have added a test for this.

Comment 17

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebbcf3fc9240

Comment 18

[Mounir Lamouri (:mounir)]

This has been merged to m-c 5 days ago. For same reason, the merge tool failed to mark the bug as fixed.
https://hg.mozilla.org/mozilla-central/rev/ebbcf3fc9240

Comment 19

[Ed Morley [:emorley]]

(In reply to Mounir Lamouri (:mounir) from comment #18)
> This has been merged to m-c 5 days ago. For same reason, the merge tool
> failed to mark the bug as fixed.
> https://hg.mozilla.org/mozilla-central/rev/ebbcf3fc9240

Because it uses the first bug number in the commit message, which was bug 230474.

As for why bug 230474 itself only had the comment added, and not resolved fixed - I can only think that RyanVM overrode the "please mark this as resolved" checkbox. 

Ryan, was this the case?

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Can't say I remember at this point. Probably something silly like that.