>>> My Info: Win7_64, Nightly 46, 32bit, ID 20160105030211 STR: 1. Open attached "testcase 1", make sure that cursor is blinking at the start of 1st line 2. Press End to move cursor to the end of the line 3. Press Shift+Right to select "\n" 4. Press Right key to move cursor to the end of text selection 5. Type "x" Result: After Step 4 cursor is displayed at the end of the 1st line After Step 5 the typed letter ("x") appears in the beginning of the 2nd line Expectations (either A or B): A) After Step 4 cursor should be displayed at the start of the 2nd line B) After Step 5 the typed letter ("x") should appear in the end of the 1st line

Regression pushlog against A): https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b8041e7ee525&tochange=fa1ea174e770 Triggered by: Bug 1048752

Looks like step 3 (shift right) to select "\n" gets things confused.

Sorry, the regression range isn't correct. I tried the following: 34.0a1 (2014-08-01) 34.0a1 (2014-08-15) 34.0a1 (2014-08-16) 34.0a1 (2014-08-17) They *all* show the bug: Caret displayed at the end of the first line, x inserted on the second line. I noticed a small change in behaviour between the 16th and the 17th. The versions of the 1st, 15th and 16th all place the caret onto the second line for a split second, but then it moves to the end of the first line. The version of the 17th doesn't show that effect. There the caret is put at the end of the first line to start with. BTW, A) is the correct expected outcome to be consistent with IE and Chrome. BTW, those both show a non-collapsed selected (looks like a selected blank) when doing the "shift right". In FF the caret disappears and no selection is shown which isn't so nice either. I took the liberty do adjust the summary and added: Caret out of sync with collapsed selection. (Collapsed selection means that the start and the end of the selection are the same).

BTW: Searching for "caret" in Bugzilla currently gives more than 300 bugs, more than 50 have the word "position" as well.

(In reply to Jorg K (GMT+1) from comment #4) > I noticed a small change in behaviour between the 16th and the 17th. I'm pretty sure that's what Alice meant: it became worse (a little). Anyway, I just tested Steps 1-4 with "Caret browsing" on a normal text, and it shows the same bug. It's reproducible since bug 859088 So here are my options (if nobody stops me): 1) To set "Component" -> "Editor" and set this as blocking bug for 859088 [OR] 2) To file new bug "Core -> Editor" with all info above, and set as blocking for 859088 and this bug Do you have other suggestions?

Please file a bug.

I did the changes you suggested. Yes, this seem to have something to do with bug 859088. I will back out that change locally and see what happens. The behaviour of the system depends on the setting of layout.selection.caret_style, for 0 and 2 it's bad as described here, for 1 and 3 it's different (also no good, since no cursor is displayed). My suggestion is to *not* file a new bug (why should we?). I will confirm the regression of bug 859088 once my compile is done. I will also test bug 1237286. Core::Editor is an unowned module, so the chances are slim that this will get fixed.

(In reply to Alice0775 White from comment #7) > Please file a bug. Please don't. We have enough bugs in the system. Unless someone who analyses this decides that there is a second issue that needs to be treated elsewhere, there is no need for two independent bugs.

Looking at bug 1153237, bug 1077515, I think "Component" should be set to "Selection". I asked this because I can never be sure about component.

OK, when I take the relevant change from bug 859088 out (https://hg.mozilla.org/mozilla-central/rev/9a09523eb5dd) which is a one line change, the behaviour indeed changes for layout.selection.caret_style = 0. This then starts behaving like setting 1 or 3, which is still incorrect, since it should place the caret at the start of the line, not behind the first character. In any case, the bad behaviour can still be observed for setting 2. IMHO, this is just another one of the 300+ caret problems ;-(

Mats, there is an implication that this is your regression. Can you handle it?

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #12) > Mats, there is an implication that this is your regression. Not really, the bug was already there as you can see from this testcase. (the bug is in Selection.collapse()) Bug 1237236 made us take a path that leads to a collapse() call here: http://hg.mozilla.org/mozilla-central/annotate/f0c0480732d3/layout/generic/nsSelection.cpp#l944

All that bug 859088 did was to use caret style 2 instead of caret style 0 in one case: https://hg.mozilla.org/mozilla-central/rev/9a09523eb5dd#l1.21 Personally I don't understand what the different caret styles are about and the description is hard to understand, if at all: https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#2271 As per my comment #11, *all* the caret styles have a problem in this particular testcase, bug 859088 only shifted the problem from one problematic behaviour to another. IMHO the major problem is that when selecting the "\n" at the end of the line, the cursor disappears altogether. This is not how other browsers behave. They display a selected space at the end of the line to show the user that some white-space was selected. Also, the caret style behaviour seems to be in need of a cleanup. Looking at https://wiki.mozilla.org/Modules/Core, Core::Selection is also unowned, and even writing to Mitchell Baker hasn't fixed that yet: https://groups.google.com/forum/#!topic/mozilla.governance/-0sAFE-_Us4

All the orange is unrelated afaict, https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cc07036e3f1

Comment on attachment 8730940 [details] [diff] [review] fix This works for me. Would you like to have a look at bug 1237286 while you have the box open ;-) Looks like a change I made in bug 756984 exposed another problem. There is some analysis in bug 1237286 comment #10. The problem looks similar to this bug. After some action, the caret is placed on the previous line where it should go onto the next line.

Comment on attachment 8730940 [details] [diff] [review] fix I have a bit long review queue right now. Maybe ehsan can help here.

Comment on attachment 8730940 [details] [diff] [review] fix Review of attachment 8730940 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsSelection.cpp @@ +4940,5 @@ > + if (aParentNode.IsContent()) { > + nsTextFrame* f = do_QueryFrame(aParentNode.AsContent()->GetPrimaryFrame()); > + if (f && f->IsAtEndOfLine() && f->HasSignificantTerminalNewline()) { > + int32_t start, end; > + f->GetOffsets(start, end); Nit: you can use GetContentEnd() instead, no need to also retrieve the start offset. @@ +4942,5 @@ > + if (f && f->IsAtEndOfLine() && f->HasSignificantTerminalNewline()) { > + int32_t start, end; > + f->GetOffsets(start, end); > + if (end == int32_t(aOffset)) { > + mFrameSelection->mHint = CARET_ASSOCIATE_AFTER; Nit: please use SetHint().

Comment on attachment 8730941 [details] [diff] [review] tests Review of attachment 8730941 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/tests/test_reftests_with_caret.html @@ +170,5 @@ > tests.push([ 'bug966992-2.html' , 'bug966992-2-ref.html' ]); > tests.push([ 'bug966992-3.html' , 'bug966992-3-ref.html' ]); > tests.push(function() {SpecialPowers.pushPrefEnv({'clear': [['layout.css.overflow-clip-box.enabled']]}, nextTest);}); > } else { > + is(SpecialPowers.getIntPref("layout.spellcheckDefault"), 0, "Spellcheck should be turned off for this platform or this if..else check removed"); Hmm, what changed on this line?

> Hmm, what changed on this line? Just a typo: s/platrom/platform/

https://hg.mozilla.org/integration/mozilla-inbound/rev/f245074b4fb4 https://hg.mozilla.org/integration/mozilla-inbound/rev/b3b6173bf031

https://hg.mozilla.org/mozilla-central/rev/f245074b4fb4 https://hg.mozilla.org/mozilla-central/rev/b3b6173bf031

Scenario in comment 0 looks fixed on: Win7_64, Nightly 48, 32bit, ID 20160319030558 But I still can reproduce the same issue. Should I file new bug?

Yes, please file a new bug with testcase + steps to reproduce.