in a pre filled textbox, cursor is behind all text, previously it was on the first position

VERIFIED FIXED in Firefox -esr52

Status

()

P1
normal
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: carlos, Assigned: farre, NeedInfo)

Tracking

({regression})

51 Branch
mozilla55
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 wontfix, firefox52- wontfix, firefox-esr5253+ verified, firefox53+ verified, firefox54+ verified, firefox55 verified)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160123151951

Steps to reproduce:

Opened our internal web based e-mail service with new firefox...


Actual results:

After upgraded to 51.01 In a prefilled textbox ( a reply ), cursor is behind all text.


Expected results:

Cursor should be on the first position, as it was for years...

Comment 1

2 years ago
It's due to bug 1287655.
Blocks: 1287655
Component: Untriaged → DOM: Core & HTML
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All
Version: 44 Branch → 51 Branch
(Reporter)

Comment 2

2 years ago
Very interesting, on the site cited in that bug
https://www.fxsitecompat.com/en-CA/docs/2016/caret-will-be-placed-at-the-end-when-textbox-automatically-gets-focus/
They say, "they follow the HTML spec". But where is this in the "HTML spec" ? And in which one?
(Reporter)

Comment 4

2 years ago
I think the selection start, selection end is not the same as where is a cursor located by default in textarea. Other browsers still works fine.
So as I can see Firefox solved a problem with selection position return by a new bug - instead to count the text correctly, ff is just placing the cursor at the end by default. Still can't see this (default cursor position) in any specification btw.!

Comment 5

2 years ago
This has created a problem for us as well, both for our webmail client and for content management system.  Would it be possible to add an option to allow for the old behavior?

Comment 6

2 years ago
We also get support requests from our customers. Could you please restore the old behavior? Thanks and bye, Thomas
Decky, are you able to comment here? Or maybe smaug who reviewed your patch?
Flags: needinfo?(coss)
Flags: needinfo?(bugs)

Comment 8

2 years ago
This new behaviour is highly problematic for our webmail service, users are complaining why is now cursor placed at the bottom of all email text when replying and why they have to scroll back to beginning each time. In other browsers, the cursor is always at the beginning.

Happens in all Firefox 51 versions (Windows/Linux/Mac).

Please consider reverting this change, it's highly annoying and we're getting lots of complaints.

Thanks,
Petr.

Comment 9

2 years ago
ehsan, as smaug didn't reply to the NI, could you give your opinion about this issue? It seems to affect some users.
Flags: needinfo?(ehsan)
(FWIW, this is still in my todo list, but atm there are plenty of review requests to deal with first)

Comment 11

2 years ago
Is there a test case?  As far as I can tell all of the complaints here are about internal webmail services and whatnot, so I'm not sure how I can help without being able to test what the problem is.
Flags: needinfo?(ehsan)

Comment 12

2 years ago
(In reply to :Ehsan Akhgari from comment #11)
> Is there a test case?  As far as I can tell all of the complaints here are
> about internal webmail services and whatnot, so I'm not sure how I can help
> without being able to test what the problem is.

Yes, see https://jsfiddle.net/zevp3dpa/1/ from https://bugzilla.mozilla.org/show_bug.cgi?id=1334723#c5

Comment 13

2 years ago
(In reply to Loic from comment #12)
> (In reply to :Ehsan Akhgari from comment #11)
> > Is there a test case?  As far as I can tell all of the complaints here are
> > about internal webmail services and whatnot, so I'm not sure how I can help
> > without being able to test what the problem is.
> 
> Yes, see https://jsfiddle.net/zevp3dpa/1/ from
> https://bugzilla.mozilla.org/show_bug.cgi?id=1334723#c5

I don't understand...  On this test case, both Chrome and Safari put the cursor at the beginning, but on this one (bug 1287655 comment 1) they both put the cursor at the end.  What is the difference between the two?

Comment 14

2 years ago
Huh, it seems like the difference in behavior depends on whether the value of the text control has been changed by script or not.  On this test case we all behave the same way: <https://jsfiddle.net/2fuatdng/>

Updated

2 years ago
Duplicate of this bug: 1334723

Comment 16

2 years ago
We should fix this ASAP, this probably occurs on piles on websites.  Andrew, can you please find an owner?

My guess on how to fix this would be to check whether the value has come from the user around here <https://hg.mozilla.org/mozilla-central/rev/52a0d2d76397#l13.20> and decide whether to use 0 or |length| accordingly and then fix all of the tests which will break as a result.
Status: UNCONFIRMED → NEW
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox54: --- → affected
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
tracking-firefox54: --- → ?
Ever confirmed: true
Keywords: regression

Updated

2 years ago
Flags: needinfo?(overholt)
Flags: needinfo?(coss)
Flags: needinfo?(bugs)
Andreas said he could take this.
Assignee: nobody → afarre
Flags: needinfo?(overholt)
Priority: -- → P1
farre, if you don't get to this soon, ping me. I could (should?) close my review queue for some time.
(Assignee)

Comment 19

2 years ago
Created attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

This seems to work, but I haven't checked all test cases changed by Bug 1287655 yet. Some seems to pass and some seems to fail. Fiddles in comments behave as expected though.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7f6291e631797f17870c8576a82fe97a5872487
Attachment #8842050 - Flags: review?(bugs)
Too late for 51 and we've built 52 RC. Mark 51 won't fix.
status-firefox51: affected → wontfix
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

We have still some issues dealing with selectionStart/End with display:none elements, but those are separate issues.

Because ValueChanged flag is cleared when reset happens, I thought this would affect to this, but looks like no.

I'd like to see which all tests need to be changed and how.
Attachment #8842050 - Flags: review?(bugs) → review+
(Assignee)

Comment 22

2 years ago
Created attachment 8842468 [details] [diff] [review]
0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch

First batch of fixed tests. Basically either calling setSelectionRange on input/textarea element before calling focus (on elements with an unchanged value, that is) or changing expected result to have prepended characters instead of appended.

Pushed another try build to look for further errors: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad12dba4d4eeab0a4cc8604799780f278faab922
(Assignee)

Comment 23

2 years ago
Created attachment 8843327 [details] [diff] [review]
0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch

Another bunch of failed tests fixed.

Still tests failing on try, but I'm not sure if this fix is to blame. It's a bit messy.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5e0f408773a5e214d22cf11d1df56fad2b9ce23&selectedJob=81444715
Attachment #8842468 - Attachment is obsolete: true
Attachment #8843327 - Flags: review?(bugs)
So some changes are such which bug 1287655 didn't change.
Could you explain those.
Comment on attachment 8843327 [details] [diff] [review]
0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch

But in general this seems to back out some changes.
Attachment #8843327 - Flags: review?(bugs) → review+
Could you perhaps file a followup bug related to selection handling when input uses style="display: none;"
Bug 1343037 is blocked on this, and should address the display:none issues.  The reason it's blocked on this is to avoid backport pain here around the "where is the cursor" behavior in the display:none case.
Blocks: 1343037
Comment hidden (offtopic)
Assignee: afarre → bzbarsky
Status: NEW → ASSIGNED
Assignee: bzbarsky → afarre
Comment hidden (offtopic)

Comment 30

2 years ago
Pushed by afarre@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa56e40215a6
Only place cursor at textarea/input end if content was changed. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/cce6ce016dfa
Update test cases to match new cursor behavior. r=smaug

Comment 31

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa56e40215a6
https://hg.mozilla.org/mozilla-central/rev/cce6ce016dfa
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(In reply to :Ehsan Akhgari from comment #16)
> We should fix this ASAP, this probably occurs on piles on websites.  Andrew,
> can you please find an owner?

Is this bad enough that we should be considering it for dot release inclusion? Also, I assume we're going to want some level of QA here before requesting branch uplifts?
status-firefox-esr52: --- → affected
tracking-firefox-esr52: --- → ?
Flags: needinfo?(afarre)
(Assignee)

Comment 33

2 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #32)
> (In reply to :Ehsan Akhgari from comment #16)
> > We should fix this ASAP, this probably occurs on piles on websites.  Andrew,
> > can you please find an owner?
> 
> Is this bad enough that we should be considering it for dot release
> inclusion? Also, I assume we're going to want some level of QA here before
> requesting branch uplifts?

Ehsan, did you have numbers or is it a gut estimate?
Flags: needinfo?(afarre) → needinfo?(ehsan)

Comment 34

2 years ago
(In reply to Andreas Farre [:farre] from comment #33)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #32)
> > (In reply to :Ehsan Akhgari from comment #16)
> > > We should fix this ASAP, this probably occurs on piles on websites.  Andrew,
> > > can you please find an owner?
> > 
> > Is this bad enough that we should be considering it for dot release
> > inclusion? Also, I assume we're going to want some level of QA here before
> > requesting branch uplifts?
> 
> Ehsan, did you have numbers or is it a gut estimate?

I don't have numbers, no, but the code pattern in question is extremely common...  In terms of whether we should do a dot release, the frequency of the bug is one thing to consider, the implications are another.  This will appear as annoying issues that affect usability potentially, not crashes or security issues, so I doubt that the severity justifies a dot release on its own.  It's probably worth considering taking as a ride along if we do a dot release separately.  Ultimately of course it's the release manager's call.

About QA, it's certainly worth verifying that the websites this was reported on here and in bug 1334723 aren't affected after the fix.
Flags: needinfo?(ehsan)
Flagging this for manual verification before potential uplift.
Flags: qe-verify+
Flags: needinfo?(andrei.vaida)
Track 53+/54+ as regression.
tracking-firefox53: ? → +
tracking-firefox54: ? → +
(In reply to Julien Cristau [:jcristau] from comment #35)
> Flagging this for manual verification before potential uplift.

Forwarding this to Brindusa, who's usually taking care of bug work requests on the nightly channel.
Flags: needinfo?(andrei.vaida) → needinfo?(brindusa.tot)
I verified this issue on Mac OS X 10.10, Windows 10, Ubuntu 16.04 with FF Nightly 55.0a1(2017-03-14)and I can confirm the fix. Using the test case from comment 12 I see the caret in front of the text, as expected.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
Flags: needinfo?(brindusa.tot)
Andreas, can you request uplift? We can give this a try on beta so that it will be fixed in 53 (current release date will be April 18th. )   If it looks good on beta then we can consider it in case of a 52 dot release too.
Flags: needinfo?(afarre)
(Assignee)

Comment 40

a year ago
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1287655

[User impact if declined]:
Annoying behaviour that potentially affects usability and that is a divergence from other browsers and older versions of gecko. 

[Is this code covered by automated tests?]:
Yes, see patch 0002

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
STR as in comment 12 and bug 1334723

[List of other uplifts needed for the feature/fix]:
Only the patches attached to this bug.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Small change.

[String changes made/needed]:
None
Flags: needinfo?(afarre)
Attachment #8842050 - Flags: approval-mozilla-beta?
(Assignee)

Comment 41

a year ago
Comment on attachment 8843327 [details] [diff] [review]
0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch

Approval Request Comment
See comment 40
Attachment #8843327 - Flags: approval-mozilla-beta?
(Assignee)

Comment 42

a year ago
Caveat: this is my very first uplift request! Please tell me if somethings missing!
Attachment #8842050 - Flags: approval-mozilla-aurora?
Attachment #8843327 - Flags: approval-mozilla-aurora?
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

Fix a usability regression and was verified. Beta53+ & Aurora54+.
Attachment #8842050 - Flags: approval-mozilla-beta?
Attachment #8842050 - Flags: approval-mozilla-beta+
Attachment #8842050 - Flags: approval-mozilla-aurora?
Attachment #8842050 - Flags: approval-mozilla-aurora+
Attachment #8843327 - Flags: approval-mozilla-beta?
Attachment #8843327 - Flags: approval-mozilla-beta+
Attachment #8843327 - Flags: approval-mozilla-aurora?
Attachment #8843327 - Flags: approval-mozilla-aurora+
Andreas, care to request uplift to release/esr52?
Flags: needinfo?(afarre)
(Assignee)

Comment 47

a year ago
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

[Feature/Bug causing the regression]:
Bug 1287655

[User impact if declined]:
Annoying behaviour that potentially affects usability and that is a divergence from other browsers and older versions of gecko.

[Is this code covered by automated tests?]:
Yes, see patch 0002

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
STR as in comment 12 and bug 1334723

[List of other uplifts needed for the feature/fix]:
Only the patches attached to this bug.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Small change.

[String or UUID changes made/needed]:
None
Flags: needinfo?(afarre)
Attachment #8842050 - Flags: approval-mozilla-release?
Attachment #8842050 - Flags: approval-mozilla-esr52?
(Assignee)

Comment 48

a year ago
Comment on attachment 8843327 [details] [diff] [review]
0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch

See comment 47.
Attachment #8843327 - Flags: approval-mozilla-release?
Attachment #8843327 - Flags: approval-mozilla-esr52?
I have reproduced this issue using Firefox 52.0.1 (ID=20170316213829) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 53.0b5, 54.0a2 on Win 8.1 x64 and Mac OS X 10.11 and Ubuntu 16.04 x64.
status-firefox53: fixed → verified
status-firefox54: fixed → verified
Flags: qe-verify+
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

let's get this in esr52 (default branch) at least, I'm still on the fence about dot release inclusion.
Attachment #8842050 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Attachment #8843327 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Comment on attachment 8842050 [details] [diff] [review]
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

I'm going to call 53 and 52.1.0esr good enough.
Attachment #8842050 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #8843327 - Flags: approval-mozilla-release? → approval-mozilla-release-
status-firefox52: affected → wontfix
(Assignee)

Comment 54

a year ago
(In reply to Wes Kocher (:KWierso) from comment #53)
> I had to back this out of esr52 for failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=85754935&repo=mozilla-
> esr52
> 
> https://hg.mozilla.org/releases/mozilla-esr52/rev/c4f5a1f6d676

Right, those kinds of failures are expected. As far as I can tell, dom/browser-element/mochitest/test_browserElement_inproc_SetInputMethodActive.html isn't on Nightly, so I guess that it got removed somewhere along the line?

Fixing that test case is just a matter of updating expectations.
Flags: needinfo?(afarre)
(In reply to Andreas Farre [:farre] from comment #54)
> (In reply to Wes Kocher (:KWierso) from comment #53)
> > I had to back this out of esr52 for failures like
> > https://treeherder.mozilla.org/logviewer.html#?job_id=85754935&repo=mozilla-
> > esr52
> > 
> > https://hg.mozilla.org/releases/mozilla-esr52/rev/c4f5a1f6d676
> 
> Right, those kinds of failures are expected. As far as I can tell,
> dom/browser-element/mochitest/
> test_browserElement_inproc_SetInputMethodActive.html isn't on Nightly, so I
> guess that it got removed somewhere along the line?
> 
> Fixing that test case is just a matter of updating expectations.

Can we get a new patch with expectations fixed up for esr52 then?
status-firefox-esr52: fixed → affected
tracking-firefox52: ? → -
Flags: needinfo?(afarre)
(Assignee)

Comment 56

a year ago
Created attachment 8855750 [details] [diff] [review]
0001-Bug-1337392-Fix-esr52-specific-test-cases.-r-baku.patch

There were two test cases that needed fixing in esr52:

devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js
dom/browser-element/mochitest/browserElement_SetInputMethodActive.js

Both were of the sort that they needed their results checked at the front (#0hello, #0#1hello vs hello#0, hello#0#1 and 21 vs 12 respectively).
Flags: needinfo?(afarre)
Attachment #8855750 - Flags: review?(amarchesini)
Attachment #8855750 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 57

a year ago
Comment on attachment 8855750 [details] [diff] [review]
0001-Bug-1337392-Fix-esr52-specific-test-cases.-r-baku.patch

[Approval Request Comment]

If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Needed to fix test cases broken by uplifting 0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch

User impact if declined: 
0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch can't land on esr52.

Fix Landed on Version:
N/A. Needed to land 0001-Bug-1337392-Only-place-cursor-at-textarea-input-end-.patch on esr52

Risk to taking this patch (and alternatives if risky): 
Small, only changes test cases.

String or UUID changes made by this patch: 
None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8855750 - Flags: approval-mozilla-esr52?
Comment on attachment 8855750 [details] [diff] [review]
0001-Bug-1337392-Fix-esr52-specific-test-cases.-r-baku.patch

No need to get approval on test-only changes. Thanks for doing the rebase, we'll get it landed today!
Attachment #8855750 - Flags: approval-mozilla-esr52?
Timea, could you please verify this on 52.1esr as well?
Flags: needinfo?(timea.zsoldos)
tracking-firefox-esr52: ? → 53+
I can confirm this issue is fixed, I verified using Firefox 52.1esr on Win 8.1 x64 and Mac OS X 10.11 and Ubuntu 14.04 x64.
status-firefox-esr52: fixed → verified
Flags: needinfo?(timea.zsoldos)

Comment 62

a year ago
Is this supposed to be fixed for everyone in all cases?

I'm still experiencing the problem in Windows 7, using Firefox 54.0 (32-bit).
Hi Leif,

I verified on Win 7 x86, Firefox 54.0 build 32 bit (buildID: 20170608105825),with link https://jsfiddle.net/zevp3dpa/1/ (comment 12), the cursor is focused on the first position.
Could you try again and confirm if I am wrong?

Thanks, 
       Timea
Flags: needinfo?(velcroleaf)
You need to log in before you can comment on or make changes to this bug.