Closed Bug 352759 Opened 17 years ago Closed 17 years ago
[FIX] Right arrow key (to collapse selection to right) does not scroll textbox to show caret
Steps to reproduce: 1. Load a page with a long URL. 2. Press Cmd+L. 3. Press Right. or 1. Load data:text/html,<input size="8" value="aaaaaaa bbbbbbbb"> 2. Click in the textbox. 3. Press Cmd+A. 4. Press Right. Result: The textbox scrolls right by a pixel or so, but not nearly enough to make the caret visible. Expected: Should scroll all the way right. This bug makes it difficult to see or edit the end of a long URL.
Related to crash bug 352520?
No, that bug is related to the backspace/delete key only.
This regressed on trunk between 2005-10-16 and 2005-10-17, most probably due to the fix for bug 299417. Note that that patch landed on the 1.8 branch as well, and the regression is indeed also on the branch.
I get the same regression range so bug 299417 seems the likely culprit. Backing out that change does not fix it though, so there must have been some other change since then that also affect the same functions... Note that there is a separate bug, but with the same effect, on Windows and Linux - regression range for that is 2006-09-11-01 -- 2006-09-12-01 (bug 300131?) The Mac problem (or rather, the caret_style=2 problem) is that nsTypedSelection::GetSelectionRegionRectAndScrollableView() always scrolls a non-#text node to the left frame edge. http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsSelection.cpp&rev=3.266&root=/cvsroot&mark=7096,7174-7183#7151 I don't understand how GetSelectionRegionRectAndScrollableView() could have ever worked (before bug 207623 that is, when the Mac specific code was still there)
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
Summary: Right arrow key (to collapse selection to right) does not scroll textbox to show caret → [FIX] Right arrow key (to collapse selection to right) does not scroll textbox to show caret
Make sure we have an up-to-date |mHint| before scrolling, which uses it to determine which frame edge to scroll to. Also fix the caret_style=1 regression mentioned in the last comment by making yet another special case in the "PeekOffset failed" else-clause. I have tested this patch on Mac, Windows and Linux with caret_style=1/2 (all six combinations).
Attachment #240777 - Flags: superreview?(roc) → superreview+
Sorry, but due to my "mostly gone" status, I'm unlikely to be able to give this a proper review for the next two weeks, at least. I've been bitten before by giving r+'s based on "this looks OK", so I'd rather not do it here (although it does look OK, from what I've seen). Mats, could you find another candidate to review this (smontagu or roc perhaps)? Or could someone volunteer? Otherwise, it'll have to wait until I have the time (and environment) to do this properly.
Attachment #240777 - Flags: review?(uriber) → review?(roc)
Comment on attachment 240777 [details] [diff] [review] Patch rev. 1 Uri would be a better reviewer, by far, but we have to make forward progress.
Attachment #240777 - Flags: review?(roc) → review+
Checked in to trunk at 2006-11-09 08:37 PDT. -> FIXED
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
On windows FF2 looks ok going right (to me, anyway). But if I try to left arrow in a selection the caret ends up one character before the end rather than at the beginning of the selection. On Mac jay reports FF2 behaves as claimed in this bug, and left arrow correctly moves to the beginning of the selection. We won't block on a polish bug (though I suppose this has accessibility implications). Ask for approval if this patch is appropriate for the branches and we'll consider it.
You need to log in before you can comment on or make changes to this bug.