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...

It's due to bug 1287655.

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?

See https://bugzilla.mozilla.org/show_bug.cgi?id=1287655#c0

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.!

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?

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?

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.

ehsan, as smaug didn't reply to the NI, could you give your opinion about this issue? It seems to affect some users.

(FWIW, this is still in my todo list, but atm there are plenty of review requests to deal with first)

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.

(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

(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?

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/>

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.

Andreas said he could take this.

farre, if you don't get to this soon, ping me. I could (should?) close my review queue for some time.

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

Too late for 51 and we've built 52 RC. Mark 51 won't fix.

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.

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

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

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.

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.

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

https://hg.mozilla.org/mozilla-central/rev/fa56e40215a6 https://hg.mozilla.org/mozilla-central/rev/cce6ce016dfa

(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?

(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?

(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.

Flagging this for manual verification before potential uplift.

Track 53+/54+ as regression.

(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.

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.

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.

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

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

Caveat: this is my very first uplift request! Please tell me if somethings missing!

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+.

https://hg.mozilla.org/releases/mozilla-aurora/rev/196c23408253 https://hg.mozilla.org/releases/mozilla-aurora/rev/e11071d8fb7d

https://hg.mozilla.org/releases/mozilla-beta/rev/505f5940b4a4 https://hg.mozilla.org/releases/mozilla-beta/rev/4a41fe2e0a5a

Andreas, care to request uplift to release/esr52?

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

Comment on attachment 8843327 [details] [diff] [review] 0002-Bug-1337392-Update-test-cases-to-match-new-cursor-be.patch See comment 47.

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.

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.

https://hg.mozilla.org/releases/mozilla-esr52/rev/cf8216fc3b25 https://hg.mozilla.org/releases/mozilla-esr52/rev/36953ede0e59

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.

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

(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.

(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?

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).

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.

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!

https://hg.mozilla.org/releases/mozilla-esr52/rev/4ab6d5c43036 https://hg.mozilla.org/releases/mozilla-esr52/rev/13f2d85da9a7

Timea, could you please verify this on 52.1esr as well?

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.

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