Closed Bug 188440 Opened 22 years ago Closed 22 years ago

textfields clobber selection when they should not.

Categories

(Core :: DOM: Editor, defect, P1)

x86
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla1.3beta

People

(Reporter: sharding, Assigned: kinmoz)

References

Details

(Keywords: dataloss, regression, Whiteboard: editorbase)

Attachments

(1 file)

User-Agent: Mozilla/5.0 (X11; U; Linux i386; en-US; rv:1.3b) Gecko/20030109 Build Identifier: Mozilla/5.0 (X11; U; Linux i386; en-US; rv:1.3b) Gecko/20030109 This might be like Bug 87410, but that's been aroud for a long time and this particular problem just appeared for me. Somewhere in the last month or two, something changed about how copy/paste under X11 works in the URL bar. This had made it frustrating to try to paste a URL into a location bar with another page's URL in it. IMHO, this is, at its core, a problem with X11's stupid copy/paste mechanism, but that's not likely to change soon so... In the past, when I wanted to paste a completely new URL into the URL bar, I'd do either: Method A 1. Ctrl-l to select the URL bar 2. backspace to delete the existing text 3. Middle click to paste in the new URL or Method B 1. Right click in URL bar 2. ctrl-a to select all old URL text 3. Backspace to delete existing text 4. Middle click to paste the new URL This used to work fine. Now (in build 2003010908 on FreeBSD in Windowmaker), it doesn't. In Method A, 'ctrl-l' selects all of the text in the URL bar (just like it used to), but now that text is copied into the paste buffer, overwriting the new URL I had just intended to paste in. Similarly, in Method B, the old text is selected when I ctrl-a and it goes into the paste buffer. In the old behavior, the ctrl-l/ctrl-a selection allowed me to delete the text but did not copy the text to the clipboard. It's conceivable that this change happened as the result of a Bug complaining that any type of selection should result in a copy (I didn't find that bug, but I may have not searched for the right thing). But for me at least, this makes pasting URLs into Mozilla a HUGE pain in the neck. Yes, there are workarounds (the current one I'm using is: *carefully* right click in URL bar, ctrl-a, ctrl-k, paste), but this basic functionality shouldn't require workarounds, IMHO. Reproducible: Always Steps to Reproduce: 1. Copy a URL from another app 2. Ctrl-l in mozilla 3. Backspace to delete URL bar text 4. Paste clipboard into URL bar Actual Results: The text that was previously in the URL bar is pasted back in. Expected Results: The text that was selected in the external app should be pasted in.
> Yes, there are workarounds The easiest one is to just middle-click on the page (not on a link, though). We will load whatever url is in the clipboard. > 2. ctrl-a to select all old URL text This should be selecting and copying the text, since it's an explicit "select the text" action and people complained that it did not copy, as you surmised. It's been doing this for a while now. > 1. Ctrl-l to select the URL bar This should be selecting the text but _not_ copying it. It works correctly in a 2003-01-08-21 nightly, but is broken in 2003-01-09-08. The same bug triggers when tabbing into textfields; we used to highlight the text in the texfield but _not_ copy it; now we copy. This regressed as a result of the fix for bug 88049 -- backing out that patch fixes that issue.
Assignee: asa → jfrancis
Severity: minor → major
Status: UNCONFIRMED → NEW
Component: Browser-General → Editor: Core
Ever confirmed: true
Flags: blocking1.3b?
Keywords: regression
QA Contact: asa → sujay
Summary: Inconvenient URL bar selection with copy/paste in X11 → textfields clobber selection when they should not.
to kin.
Assignee: jfrancis → kin
> This should be selecting and copying the text, since it's an explicit "select > the text" action and people complained that it did not copy, as you surmised. > It's been doing this for a while now. Makes sense. No problem. (In fact, I had been annoyed by the fact that it didn't copy in the past; I just eventually learned to deal with it and then took advantage of it).
Alright. Selecting text with keyboard _also_ clobbers the selection. This is total dogfood; adding dataloss keyword (because I keep losing things I need to paste somewhere) and upping severity. The Unix builds are basically not usable because of this bug.
Severity: major → blocker
Keywords: dataloss
kin, is this right component?
Whiteboard: editorbase
Is this expected to be fixed soon or should I downgrade to an older build? This is a huge usability problem. I was holding off on downgrading in the hopes that it would be fixed fairly quickly, but it's been broken for over a week now. I'm just trying to set my expectations appropriately.
Ok appologies, I missed bz's comment saying that 88049 was responsible for this. I'm on it.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.3beta
Hmmm how lame, you can't use the SelectAllChildren() method on nsSelection without triggering an autocopy, because the autocopy listener specifically looks for a selection change of SELECTALL_REASON ... instead you have to manually add a range that spans all the children, just so you won't get a SELECTALL_REASON. <sigh> For those wondering what change in the patch for bug 88049 regressed this, it was this little tidbit: - SelectAllContents(); + if (mEditor) + mEditor->SelectAll(); SelectAllContents() was a private utility method that called SetSelectionRange(0, eSelectToEnd) which I thought was silly since that caused us to go through all the pain of converting string indexes into dom points. Looks like I'll have put back SelectAllContents() just to get around this bug, but this time just implement it to creat a range of [(div,0), (div, numchildren)] and manually add it to the selection. <sigh>
Just as an FYI, the check for SELECTALL_REASON in nsAutoCopy was added for bug 70519.
Btw, restoring the SelectAllContents() method would be a quick *temporary* solution to this bug. What really needs to happen is that there needs to be a distinction between a selection that is made programitcally and one that is made because of a user-gesture. The autocopy code keying off of a generic selection change reason is just lame.
Attached patch Patch Rev 1Splinter Review
Ok this patch adds SelectAllContents(), and tries to share some code with SetSelectionEndPoints() to set the selection range. The soonest I can check this in will be Tuesday morning, assuming I get all the required reviews etc, so if someone wants to drive this in over the weekend, be my guest. Otherwise, I'll do it.
Comment on attachment 111877 [details] [diff] [review] Patch Rev 1 Requesting reviews from the same reviewers for bug 88049.
Attachment #111877 - Flags: superreview?(sfraser)
Attachment #111877 - Flags: review?(jkeiser)
Comment on attachment 111877 [details] [diff] [review] Patch Rev 1 + nsCOMPtr<nsIDOMRange> range; + + range = do_CreateInstance(kRangeCID); You could do this in one line. Looks good otherwise.
Attachment #111877 - Flags: superreview?(sfraser) → superreview+
Flags: blocking1.3b? → blocking1.3b+
Comment on attachment 111877 [details] [diff] [review] Patch Rev 1 r=brade
Attachment #111877 - Flags: review?(jkeiser) → review+
Fix checked into the TRUNK: mozilla/layout/html/forms/src/nsTextControlFrame.cpp revision 3.113 mozilla/layout/html/forms/src/nsTextControlFrame.h revision 3.47
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Btw, I checked in a version of the fix with sfraser's suggestion.
*** Bug 190018 has been marked as a duplicate of this bug. ***
Keywords: nsbeta1+
*** Bug 191603 has been marked as a duplicate of this bug. ***
here's how i tested this (2003.02.19, linux rh8.0): 1. copied a url (context menu "copy link location" for link in either mailnews or browser). 2. ctrl-L to select urlbar contents. 3. hit backspace to delete urlbar contents. 4. pasted (1) via either ctrl-V or middle-click in urlbar. results: the copied url from step 1 was successfully pasted. marking vrfy'd.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: