Closed Bug 829606 Opened 7 years ago Closed 7 years ago

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

Categories

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

1.0 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: libreman.bugzilla, Assigned: mounir)

References

Details

Attachments

(2 files)

Attached file 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.
Attachment #701103 - Attachment mime type: text/plain → text/html
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Component: DOM: Core & HTML → Editor
Whiteboard: DUPEME
OS: Linux → All
Hardware: x86_64 → All
Is this a regression?
FWIW, Firefox 17 is affected.
Version: 19 Branch → 17 Branch
(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
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?
Keywords: qawanted
It seems this goes way back to 1.0 according to Bug 230474
(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.
Version: 17 Branch → 1.0 Branch
Hi, is anything going to happen with this bug? Is there any chance somebody might work on it? Thanks.
Attached patch PatchSplinter Review
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #714984 - Flags: review?(bzbarsky)
Flags: in-testsuite+
Keywords: qawanted
Thanks! It's super awesome that you responded so fast, I hope the patch will be accepted ;)
Mounir, have you checked why that code was there?  Why is it ok to just remove it completely?
(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).
> 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.
(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 on attachment 714984 [details] [diff] [review]
Patch

Thanks for checking that.

r=me
Attachment #714984 - Flags: review?(bzbarsky) → review+
(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.
Blocks: 230474
Whiteboard: DUPEME
Component: Editor → DOM: Core & HTML
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
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(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?
Can't say I remember at this point. Probably something silly like that.
Blocks: 779913
Depends on: 893312
You need to log in before you can comment on or make changes to this bug.