Closed
Bug 28583
Opened 25 years ago
Closed 22 years ago
Focusing text widget (except on click) should select widget contents [also form <input> field]
Categories
(Core :: XUL, defect, P3)
Core
XUL
Tracking
()
VERIFIED
FIXED
mozilla1.0
People
(Reporter: mpt, Assigned: aaronlev)
References
Details
(Keywords: access, polish, Whiteboard: [behavior])
Attachments
(1 file, 13 obsolete files)
(deleted),
patch
|
aaronlev
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
Build: 2000021608, MacOS 8.6
To reproduce:
* Open the Bookmarks window.
* Open the Properties window for a bookmark.
* If there is nothing in the Location field for the bookmark (it doesn't seem to
be hooked up yet), type some text into it.
* Place the cursor in the Name field.
* Press Tab.
Or:
* Open the prefs window, and navigate to Advanced > Proxies.
* Select the `Manual proxy configuration' radio button.
* Place the cursor in the FTP proxy server name field.
* Press Tab.
What should happen (on all platforms):
* The next widget, which is a text field, is given focus and its entire contents
is selected (so that any new typing will overwrite the existing contents).
What actually happens:
* The text field is given focus, but its contents are not selected.
Comment 1•25 years ago
|
||
reproduced in recent verification build on Win98. reassigning to pollmann. Eric,
is this part of the tab mgr work I hear you are doing?
Assignee: trudelle → pollmann
to qa: when this bug is fixed, pls notify esther or lchiang so we can try our
dup bug case. Thanks.
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → M20
Comment 6•25 years ago
|
||
Minor usability issue, scheduling for M20!
Updated•25 years ago
|
QA Contact: paulmac → sujay
Comment 7•25 years ago
|
||
Not so minor, IMO. And it's all the little "minor" details that make a product
feel good. Skip the minor details and you wind up with a rather frustrating
product. Why do you think MS IE 4.5 and 5 did so well on the Mac? It
wasn't because they had Microsoft's name behind them...
Just registering my UI vote so these things don't slip through the cracks.
Comment 8•25 years ago
|
||
This bug is probably a duplicate of or at least related to bug 36278 (which has
been incorrectly classified as a duplicate of 31809 and 33069). See also 29086
Comment 10•25 years ago
|
||
Reassigning to you Steve - this looks like it's more your area.
I tried a first pass at implementing this a while back and ran into lots of
focus issue and other problems. (will attach patch, which was only a few line
change to nsGfxTextControlFrame.cpp)
1) The text was selected only the first time the widget received focus
2) The text was not deselected when the widget lost focus.
3) Clicking on an empty widget caused asserts to fire
4) Peformance of clicking on a widget dropped significantly.
Assignee: pollmann → buster
Severity: normal → enhancement
Status: ASSIGNED → NEW
OS: Mac System 8.6 → All
Hardware: Macintosh → All
Target Milestone: M20 → ---
Comment 11•25 years ago
|
||
Comment 12•25 years ago
|
||
*** Bug 36278 has been marked as a duplicate of this bug. ***
Comment 13•25 years ago
|
||
The reason for problem 2) is that nsGfxTextControlFrame::SetFocus() never gets
called with aOn false - meaning, we're never notified when the text control
looses focus. This could be fixed by just moving the Deselect() call to
wherever we get that notification.
Comment 15•25 years ago
|
||
assigning to Kin for review
Assignee: beppe → kin
Summary: Tabbing to text widget should select widget contents → [RFE]Tabbing to text widget should select widget contents
Target Milestone: --- → Future
Reporter | ||
Comment 17•25 years ago
|
||
Hey, I protest. This isn't an RFE, it's a Bug with a capital B.
W.r.t. Eric's comments -- the widget contents should only be selected when the
widget is tabbed to, *not* when it is clicked in. Clicking should just position
the caret, as it would if the widget already had focus.
Comment 18•25 years ago
|
||
Tabbing into a textfield in WinNt Nav 4.x doesn't select the content, but the
textfield does remember where the cursor was (that could be a bug)
Comment 19•25 years ago
|
||
I'd like to add that this behavior should *not* be enabled on Linux and other
X11-using platforms, as it interferes with the X selection/paste mechanism.
Reporter | ||
Comment 20•25 years ago
|
||
Decklin, does GTK really not do this when you tab to a text widget? If so, that
means pp. I think it's possible that resetting the X selection when tabbing to a
text widget is actually desirable behavior anyway.
Raising severity to Minor after rereading
<http://bugzilla.mozilla.org/bug_status.html#severity>. Nominating for RTM, as I
can't believe that a commercial product would go out the door with this unfixed.
Comment 21•25 years ago
|
||
*doublechecks*
Yes, GTK really does not do it. I probably would have complained already if it
did ;-)
Updated•24 years ago
|
Reporter | ||
Comment 23•24 years ago
|
||
Not an RFE --> removing `[RFE]' from summary. Given the number of times it
happens in normal usage, not a minor issue either --> normal.
Severity: minor → normal
Summary: [RFE]Tabbing to text widget should select widget contents → Tabbing to text widget should select widget contents
Comment 24•24 years ago
|
||
*** Bug 60610 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 25•24 years ago
|
||
Ok, refining summary based on comments in bug 63705.
The contents of a text field should be selected if the field receives focus by
any method *except* clicking in it (clicking in it should just place the caret).
This includes tabbing to the field, and setting focus to the field when the
dialog is created.
Summary: Tabbing to text widget should select widget contents → Focusing text widget (except on click) should select widget contents
Reporter | ||
Comment 26•24 years ago
|
||
*** Bug 66284 has been marked as a duplicate of this bug. ***
Comment 27•24 years ago
|
||
*** Bug 74003 has been marked as a duplicate of this bug. ***
Comment 28•24 years ago
|
||
*** Bug 69310 has been marked as a duplicate of this bug. ***
Comment 29•24 years ago
|
||
remove milestone for reconsideration, add some keywords
note: on Linux, the correct behavior seems to be to place the caret at the end
of the edit field. Macintosh and Windows expects the fields to be selected.
As for 4.x behavior on Windows, 4.x is inconsisent (if I recall correctly). In
Composer dialogs, the tabbing among the various edit fields does a select all.
Comment 30•24 years ago
|
||
Set to mozilla0.9.1 for now. Cc'ing cmanske who had the dup of this bug.
Target Milestone: --- → mozilla0.9.1
Comment 31•24 years ago
|
||
We probably need an nsILookAndFeel entry for this. I so wish it would work on
Mac.
Comment 32•24 years ago
|
||
*** Bug 79344 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Whiteboard: [behavior]
Comment 34•24 years ago
|
||
*** Bug 73078 has been marked as a duplicate of this bug. ***
Comment 35•24 years ago
|
||
*** Bug 81377 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 36•24 years ago
|
||
*** Bug 83588 has been marked as a duplicate of this bug. ***
Comment 37•23 years ago
|
||
well, we would like to get this in for 9.2, but until bug 83496 gets resolved,
this one can't. Moving over to 1.0 but will gladly move in once that otehr bug
gets fixed
Target Milestone: mozilla0.9.2 → mozilla1.0
Comment 38•23 years ago
|
||
*** Bug 89339 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 39•23 years ago
|
||
*** Bug 62880 has been marked as a duplicate of this bug. ***
Comment 40•23 years ago
|
||
*** Bug 96652 has been marked as a duplicate of this bug. ***
Comment 41•23 years ago
|
||
*** Bug 62880 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 42•23 years ago
|
||
Removing pp keyword. I discussed this bug with someone whose specialty is the X
selection mechanism, and his response was basically `RTFICCCM'. (You can do so
at <http://tronche.com/gui/x/icccm/>.) There's nothing wrong with selecting the
contents of a text field on X when tabbing to it or programmatically focusing
it, as long as it is made the SECONDARY selection, not the PRIMARY one. The bug
where tabbing to a GTK+ text field doesn't select its contents is fixed in GTK+
2.0 <http://bugzilla.gnome.org/show_bug.cgi?id=57743>.
Keywords: pp
Comment 43•23 years ago
|
||
Bulk move of mozilla1.0 bugs to mozilla.1.0.1. I will try to pull some of these
back in if I can.
Target Milestone: mozilla1.0 → mozilla1.0.1
Comment 44•23 years ago
|
||
nominating for nsbeta1, adding access keyword
Comment 45•23 years ago
|
||
Marking nsbeta1+
Updated•23 years ago
|
Target Milestone: mozilla1.0.1 → mozilla1.0
Comment 46•23 years ago
|
||
Rod, can you take a look at this?
Comment 47•23 years ago
|
||
nsbeta1-. Engineers are overloaded with higher priority bugs.
Comment 48•23 years ago
|
||
cc marlon for UE input. Is there anyone else who could take this? It seems
like fundamental usability correctness, and may be important for accessibility
too, especially IE compatibility.
Comment 49•23 years ago
|
||
Assignee | ||
Comment 50•23 years ago
|
||
I would know how to do this without a fix for bug 83496.
It would require a modification to nsEventStateManager::ShiftFocus().
It would simply check to see if the user had moved onto something with it's own
caret, if so it would select the contents.
If people don't think that's too much of a hack I can take this one.
Comment 51•23 years ago
|
||
"had moved onto something with it's own caret"
Can you explain?
Assignee | ||
Comment 52•23 years ago
|
||
textfields and textareas are editable, and as such have their own
caret/selection that is separate from the document's caret/selection.
Comment 53•23 years ago
|
||
Be sure you test any fix with the urlbar (which does some funky things (imo) to
selection.
Can we also use ShiftFocusInternal in the case where the window just opens?
Lastly, this should be a hidden preference or similar. Unix users expect a
caret (current behavior), not a selection while Macintosh and Windows users
expect there to be a selection.
Comment 54•23 years ago
|
||
Aaron: how can you use the 'thing has caret' information to decide whether the
user just focussed it with a tab key or a click (which is what this bug is
about)?
Comment 55•23 years ago
|
||
Aaron,
I propose the patch according to your suggestion before.
Function ShiftFocus will only be called when Tab is used.
So the patches can solve the problem in Bookmark property
dialog and the preference dialog etc.
Of course I have tested the patch, and it works.
Comment 56•23 years ago
|
||
Comment on attachment 72569 [details] [diff] [review]
SelectAll When ShiftFocus
One problem with this patch is that it appears to be always doing the
select-all behavior. This needs to be platform-specific (Linux expects to have
a caret at the end of the text rather than the selectAll behavior expected on
Macintosh).
jim.song@sun.com--do you need help revising this patch to account for that? I
envision the patch will make use of a hidden preference which Linux will set
differently from other platforms. Let me know.
Attachment #72569 -
Flags: needs-work+
Comment 57•23 years ago
|
||
A few other notes/comments about the patch:
* please use consistent code style for the code you are editing
(put spaces in the same places as surrounding code)
* add a comment about what the block of code (which you are adding) is doing
* it seems to work great (hurray!) except I don't see a caret when I tab to an
empty textarea.
* Also, occasionally when I click I see a flash where the text seems to select
and then the caret is placed and the selection is removed.
steps to reproduce "flicker":
* load this bug in a browser window
* click in the QA context text field
* tab until you get to the textarea for Additional Comments
(notice there isn't a caret)
* click in a textfield which has text in it (such as blocker list)
(watch carefully for flicker)
* click in other textfields that have text in them; see flicker
Subsequent clicks have no flicker unless the field has a selection in it prior
to blur. The flicker is NOT a regression introduced by this patch.
I'm not sure about the lack of a caret in a textarea; I think that should be
addressed (as well as the preference for Linux).
Assignee | ||
Comment 58•23 years ago
|
||
Couple of notes:
Jim, Looks like great progress!
Please emember to remove the comment with your name in the final patch. Patches
that checked in shouldn't have personal notes left in them.
Be aware there may be a conflict with a patch I am checking in for bug 66597.
That patch will be checked in this week (hopefully tonight).
Comment 59•23 years ago
|
||
I'm somewhat uneasy about this patch: my feeling is that the ESM is not the
right place to be putting SelectAll() behaviour, preffed or not.
Comment 60•23 years ago
|
||
Hi, Thanks!
No Caret if blank:
My fault. Will give patch tomorrow.
Flicker:
Not introduced by this patches. So need another bug?
No Selection on Linux:
As Mattew said before, Gtk2.0 require Selection when tabbing.
http://bugzilla.gnome.org/show_bug.cgi?id=57743
Comment 61•23 years ago
|
||
I have added code to show the cursor if the content is empty.
Please review.
Attachment #8221 -
Attachment is obsolete: true
Attachment #72569 -
Attachment is obsolete: true
Comment 62•23 years ago
|
||
Comment on attachment 73178 [details] [diff] [review]
New Patch
Like sfraser, I'm a bit uneasy about placing this in ESM.
When I look at this patch, I'm wondering to myself, what happens if
mCurrentTarget isn't a text widget, what selection controller is returned? The
one for outer document? If so, will the select all cause the entire outer doc
to get selected?
Also does getting the "value" attribute on a TextArea really work? Getting the
value of a text widget as a string is not cheap. If we could just distinguish
how we got focus, nsGfxTextControlFrame or the editor could just peak at the
number of children underneath the root node.
Comment 63•23 years ago
|
||
removing myself from the cc list
Comment 64•23 years ago
|
||
Hi, Kin,
>If mCurrentTarget isn't a text widget, what selection controller is
>returned? The one for outer document?
Do you think it's right to return the document's selection controller
for a widget?
I think we can check whether it is a text widget if it is needed, but
if there is the case to tab to a document as a whole?
Comment 65•23 years ago
|
||
Sorry for my upper unclear comments.
For Kin's uneasy, I wonder if there is the case that, a control is tabbed into,
and its selection control is from the outer document(as Kin said)?
If tabbing and select only apply to text widget, I also wonder if we can check the
frame type, if it is text widget, then select it?
Assignee | ||
Comment 66•23 years ago
|
||
// The following code is a generic way to check if we're in a text field
nsCOMPtr<nsIFrameSelection> frameSelection, docFrameSelection;
GetSelection(mCurrentTarget, mPresContext, getter_AddRefs(frameSelection));
mPresShell->GetFrameSelection(getter_AddRefs(docFrameSelection));
if (docFrameSelection && (frameSelection != docFrameSelection)) {
// If we get here we are in a textfield or textarea,
// because they have their own frame selection, separate from the document's
}
Comment 67•23 years ago
|
||
Add Aaron's codes to check if it's a text field.
A small change from Aaron's is:
if( frameSelection && ... )
Comment 68•23 years ago
|
||
Mozilla should handle single-line inputs and textareas differently.
Here are some situations where selecting the entire contents of a
textarea on focus would be a bad idea:
Situation 1
1. Log in to http://slashdot.org/ or http://www.kuro5hin.org/
2. Open a story
3. Click "Reply to This" under a comment
4. Compose a reply
5. Click Preview
6. Tab to field
Situation 2
1. Log in to http://www.wikipedia.com/
2. Click "Recent Changes" to get a list of articles
3. Find an article that looks interesting and click its title
4. Click "Edit this page"
5. Tab to the edit box containing the article's data
Situation 3
1. Fill in the Bugzilla Helper form
2. Submit the form to create a new bug report
3. Tab to the textarea
Selecting the entire contents of a textarea on focus would be roughly
equivalent to selecting the entire contents of a newly opened Composer
page. I see no reason why the user would want to replace the entire
contents of a textarea with one keystroke. If the user really wants
to do that, it's as easy as Accel+A.
Comment 69•23 years ago
|
||
Yes, Danmian, I think you are quite right.
Comment 70•23 years ago
|
||
Yes, Damian, I think you are quite right.
Comment 71•23 years ago
|
||
I agree. Opera selects the contents of textareas you tab into, which seems
wrong, even though it is what you expect in a textfield. IE6 and NS4.x both put
a caret at the end of the text, which IMO is what mozilla should do too.
Comment 72•23 years ago
|
||
Best Solution:
1. Single-line inputs - entire text is selected
2. Multi-line textareas - curser placed at end of existing text
1. Allows to quickly select/replace small (default) entries. Accidental deletes
are quickly repaired (few text).
2. Allows to edit text without the risk of accidentally deleting (and then
having to rewrite) large amounts of text.
Comment 73•23 years ago
|
||
Hi,
Does selection only apply to widget with localname as:
text and input? If so, we can only select content with
such names.
Comment 74•23 years ago
|
||
nc4 remembers where in an input widget your cursor was. Eg if I click the a in
the status whiteboard [beh_avior] then tab out and shift tab back in, my cursor
is placed at beh_avior, which is what I expect. The same applies for textareas.
Assignee | ||
Comment 75•23 years ago
|
||
The CSS "display:" attribute can be used to make any element display as a text
input, so checking the frame type is the best way.
Comment 76•23 years ago
|
||
I agree with timeless. We should be able to remember where the selection was in
a textarea.
Comment 77•23 years ago
|
||
Mozilla does currently remember position and selection state when tabbing
through textboxes.
Comment 78•23 years ago
|
||
*** Bug 131185 has been marked as a duplicate of this bug. ***
Comment 79•23 years ago
|
||
I think we should deal it more easier.
Select all if it is a singel-line text field, and do nothing but leave it's old
selection and cursor position if it is a textarea.
Just like what IE does.
Comment 80•23 years ago
|
||
Just add SelectAll(aPresContext); to nsHTMLInputElement.cpp
Because it is the text field and xul input element's implementation.
I tested it on windows with pref and html form element.
This select action is only for text and password, and it will not effect the
textarea element
Attachment #73178 -
Attachment is obsolete: true
Attachment #73621 -
Attachment is obsolete: true
Comment 81•23 years ago
|
||
talking to petez on #mozilla the current patch behaves like selecting with the
mouse on X. That's bad.
The mouse selection (X clipboard?) is holy, don't mess with it without an
explicit user request.
On windows, an explicit action is needed to move the selection to the clipboard.
On X, the selection is automatically copied to the clipboard. That must not
happen as a side effect of moving focus.
Comment 82•23 years ago
|
||
Comment on attachment 74933 [details] [diff] [review]
Patch just add SelectAll in nsHTMLInputElement.cpp
This patch needs a lot more work. At a minimum *any* fix for this bug MUST
have a preference (hidden). It must default to on for Macintosh and Windows
users and off for Linux users. I expect to see a preference added to all.js as
well as the linux-specific file.
The other big problem with this patch (maybe I just need more context?) is that
it is ALWAYS does a SelectAll. That is NOT the correct behavior if one clicks
in the field (tabbing should do SelectAll; clicking should position caret).
Attachment #74933 -
Flags: needs-work+
Comment 83•23 years ago
|
||
I agree with brade. We must NOT do this on all setfocus events, i.e., when
user clicks in an input field. I would have fixed it years ago if it were that
simple! The tricky part when I first investigated this problem was how to
figure out when focus was set just by tabbing into a text input.
That is the only time we should select. And even that behavior is platform
specific, as brade points out. A pref would also be good so users could change
the behavior -- many don't like it to select at all, even if it is the
default platform behavior.
Assignee | ||
Comment 84•23 years ago
|
||
I believe I have the right fix for this bug, by implementing bug 83496.
Taking.
Assignee: rods → aaronl
Assignee | ||
Comment 85•23 years ago
|
||
-> Pete Zha
We discussed the correct fix on IRC.
Assignee: aaronl → pete.zha
Reporter | ||
Comment 86•23 years ago
|
||
As described in comment 42 and comment 60, this bug is *not* platform-specific.
That it didn't work properly in GTK 1.x was a bug which has been fixed. Like
any other selection, this selection should be copied to PRIMARY, but *not* to
CLIPBOARD.
Comment 87•23 years ago
|
||
I would like to note that the behaviour of selection and clipboard is platform
dependent. At least on solaris, you have to learn by heart which app puts it's
stuff where. As far as clipboard goes, X is not really *one* platform.
Whatever patch comes, this should get rich QA on as many X-servers as possible,
and check out different toolkits. (Adding Roland for the xlib port)
Comment 88•23 years ago
|
||
There is actually one behaviour preferred on X11 - that one of Motif2 (not the
old Motif1 one (e.g. NS4.x is Motif1.x-based)).
Ignore the rest - just implement it like Motif2/CDE2.x does the job.
Reporter | ||
Comment 89•23 years ago
|
||
> on solaris, you have to learn by heart which app puts it's stuff where.
That's not platform-dependent, that's some apps being broken. For example,
xchat still incorrectly sets CLIPBOARD on selection, while OpenOffice
incorrectly does nothing on selection and sets PRIMARY on Copy. Most other
common apps in active development have been fixed.
Comment 90•23 years ago
|
||
note: the current gtk requirements are 1.2.*, don't change that without
separate approval. I suggest making the fix for win and mac only if we can't
fix this without changing the requirements.
Reporter | ||
Comment 91•23 years ago
|
||
No, this has nothing to do with our gtk requirements. It's a bug in *Mozilla's*
text controls, which should be fixed no matter what version of gtk is
underneath. As an analogy, the Windows 95/98 native text controls have line-
-wrapping bugs which are not present in Windows 2000; that does *not* mean that
Mozilla's own text controls should emulate those bugs when on Windows 95/98.
Comment 92•23 years ago
|
||
So, Matthew: you're saying that because gtk is planning to change their behavior
in an upcoming release, which very few people have actually tried running (do
any distros ship with it yet? Redhat 7.2 uses gtk 1.2), and which mozilla
itself is not using, we should completely change the selection behavior of text
fields, making it incompatible with the ICCCM (see comment 42, where you
yourself mention that the ICCCM says tabbing into text fields should not make
the contents the primary selection). If we changed the code accordingly,
selecting the text field contents but NOT autocopying it to the clipboard, we
become compatible with the ICCCM but become very confusing for Unix users to
use, since now to get the contents into the primary selection to make it
available to paste, users will have to select it again, which isn't visually
obvious since it looks like it's already selected, and which can't be done by
the usual drag-select method since dragging inside a mozilla selection invokes
drag-and-drop code rather than selection code.
(BTW, does gtk 2.0 copy this selected text to the X primary selection? You
haven't mentioned that.)
Imposing this sort of confusion, which is seen in no other currently shipping
Unix app or toolkit, upon all Unix users for mozilla 1.0, with no prior warning
so that users have a chance to complain (which they will, I assure you) is wrong.
Reporter | ||
Comment 93•23 years ago
|
||
Ok, I made a mistake in comment 42. Sorry.
`as long as it is made the SECONDARY selection, not the PRIMARY one'
should be
`as long as it is made the PRIMARY selection, not the CLIPBOARD one'
To address your other points:
1. The GTK+ developers have already fixed this bug in GTK+ 2.0. There are *no*
plans to `change their behavior in an upcoming release'.
2. As I already said, which version of GTK Mozilla itself requires is
irrelevant to this bug, since Mozilla rolls its own UI controls, and
shouldn't replicate bugs in old versions of other toolkits.
3. Fixing this bug does not `completely change the selection behavior of text
fields', just what happens when you tab into them.
4. `Selecting the text field contents but NOT autocopying it to the clipboard'
should not be `very confusing for Unix users to use', because no app should
*ever* autocopy selected text to the clipboard. As I said, xchat is the
only major app still broken in that particular fashion.
5. Given that Mozilla 1.0 and GNOME 2.0 are likely to be released within a
couple of months of each other, and therefore distributed together in Linux
distributions, the number of people using Mozilla 1.x with GNOME 2.x will
probably be several times greater than the number of people using it with
GNOME 1.x.
> does gtk 2.0 copy this selected text to the X primary selection?
Yes it does.
Comment 94•23 years ago
|
||
Perhaps I should submit another bug report on this, but it relates rather
closely to this discussion. On MacOS X when you drag select in the URL area it
does a select-all instead of selecting the region dragged. In other words, if I
have:
http://bugzilla.mozilla.org/show_bug.cgi?id=28583
and I want to change the id to something else, so I try and drag select it, I
end up selecting the whole thing. The behavior in forms is correct, it's only in
the URL area that it's wrong. Double click behavior on the other hand, which
should select a word, behaves very oddly with URLs (whether in the URL entry
area, or a text region like this one. It seems to select from the click point
back to the first space or beginning of line, and it doesn't treat punctuation
as a word delimiter. (There are also some pending delete bugs in textareas, but
I'll check and see if they've been reported already.)
If I should submit a separate report feel free to yell at me and tell me so.
Comment 95•23 years ago
|
||
Kee: that's bug 116441.
Comment 96•23 years ago
|
||
gtk 2.0 is an upcoming release to most users, since no distro is actually
shipping apps which behave this way yet. We have no idea what user reaction to
this move will be. Most users will download mozilla 1.0 long before they
reinstall linux and see gtk 2.0, so mozilla will be where they first see this
behavior, and mozilla will appear to be broken in comparison with everything
else on their system.
> 3. Fixing this bug does not `completely change the selection behavior of text
> fields', just what happens when you tab into them.
It "completely changes the selection behavior of text fields when you tab into
them." Clearer?
> because no app should *ever* autocopy selected text to the clipboard.
I was referring to the mozilla clipboard, nsClipboard, which is the class used
for copying selections to PRIMARY as well as CLIPBOARD. Your earlier ICCCM
quote had said we shouldn't autocopy to PRIMARY, and I was responding to that.
Comment 97•23 years ago
|
||
With regard to the argument that GTK+ 1.2's behavior (no select-on-tab) is a bug:
http://www.gtk.org/gtk-2.0.0-notes.html
The new default 2.0 behavior of select-on-tab is described as a way to "improve
usability", not a bugfix. Note also that the instructions to reconfigure GTK+
back to the 1.2 behavior are included on the first page of the GTK+ 2.0 release
notes.
Summary: The "old" behavior is NOT a bug or broken. Making select-on-tab the
default behavior is fine as long as there is an obvious way to reconfigure it to
have the "old" behavior.
Comment 98•23 years ago
|
||
*** Bug 141869 has been marked as a duplicate of this bug. ***
Comment 99•22 years ago
|
||
*** Bug 155683 has been marked as a duplicate of this bug. ***
Comment 100•22 years ago
|
||
Yay! So glad I found this bug...drives me bonkers every day.
There's something related. When focusing a text field, the cursor's flashing
should start with the cursor /visible/. At the moment, when tabbing into a text
field, not only is the content not highlighted, the cursor isn't even showing.
So I don't know if I've hit tab enough times to get where I'm going. It's only
a second to wait, but if I'm tabbingn through to the next widget, there should
be visual feedback for every widget I tab to.
Does that need a separate bug...?
Assignee | ||
Comment 101•22 years ago
|
||
Mozilla@strangemonkey - I think in most programs the cursor starts as visible
right after any key is pressed,not just tab. That would be a separate bug, but
check to see if one already exists before filing a new one.
Comment 102•22 years ago
|
||
*** Bug 163232 has been marked as a duplicate of this bug. ***
Comment 103•22 years ago
|
||
Note that this only affects XUL textboxes, not editable menulists or HTML.
Comment 104•22 years ago
|
||
Comment on attachment 96297 [details] [diff] [review]
Basic XBL-based patch
brade@netscape.com commented on irc so here is some explaination:
> this.inputField.setSelectionRange(0, 0);
This stops d&d on a non-focused field which is annoys me because I can't see
what the selection is when then field isn't focused.
> if (!/Linux/.test(navigator.platform) &&
I put this test in quickly because apparently a .select() sets the clipboard in
Linux, but I'm not claiming that it's perfect code :-)
Comment 105•22 years ago
|
||
clarification: I was asking why we check for Linux before checking
this.mIgnoreFocus. For example, why not:
+ if (!this.mIgnoreFocus && !/Linux/.test(navigator.platform))
Comment 106•22 years ago
|
||
I'm not claiming perfect code (for once :-)
You might want to have a field so you can change the setting at run time:
<field name="autoSelect">!/Linux/.test(navigator.appName)</field>
And you'd probably want this in C++ to affect all users of <html:input> anyway.
Comment 107•22 years ago
|
||
Can we please also fix this for editable menulist?
Comment 108•22 years ago
|
||
Wait a minute. Doesn't this patch do exactly the wrong thing, i.e., select
everything when mouse is clicked inside input? It is supposed to select when
receiving focus by tabbing into it, but NOT when you click in the input widget.
Comment 109•22 years ago
|
||
Recent versions of gtk on linux do select all the text when you give focus to a
widget.
Comment 110•22 years ago
|
||
Do they also copy the text to the SELECTION buffer? (or whatever the buffer is
called that gets pasted on middle-click)
Comment 111•22 years ago
|
||
See comment 97, though: it's turnaffable in gtk2, it's not the default in the
gtk shipped in any currently shipping distro (i.e. real users haven't seen this
behavior yet), and it's only for tabbing in, not clicking.
Comment 112•22 years ago
|
||
"whatever the buffer is called"
You are thinking of the PRIMARY selection and it isn't a buffer, no data is sent
anywhere until you *paste* a selection. The other popular selection is the
CLIPBOARD selection which is used for the metaphor of copying / cutting /
pasting to and from an invisible clipboard.
PRIMARY is automatically asserted by the GTK+ code that deals with text
selections, so I would guess (not having ever used a GTK+2 installation where
this is enabled) that unless someone thought about this and went out of their
way to avoid it, the PRIMARY selection is changed as soon as the text appears
"selected" in the widget.
Comment 113•22 years ago
|
||
> Wait a minute. Doesn't this patch do exactly the wrong thing, i.e., select
> everything when mouse is clicked inside input? It is supposed to select when
> receiving focus by tabbing into it, but NOT when you click in the input widget.
No, the mousedown event sets the mIgnoreFocus flag so that the subsequent focus
event gets ignored. Similar code can be seen in navigator.js to handle the URL
bar (although that case has extra code to handle the click selects all pref).
Just a thought: does this have to be done in C++, or will it in fact work in
platformHTMLBindings.xml?
Comment 114•22 years ago
|
||
Patch to platformHTMLBindings.xml (Windows only, becuase that's my OS) which
fixes it for web pages, textboxes and editable menulists. I'll leave it to the
owners of the different OSes to implement it if they so desire.
Updated•22 years ago
|
Attachment #96297 -
Attachment is obsolete: true
Comment 115•22 years ago
|
||
*** Bug 170576 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Summary: Focusing text widget (except on click) should select widget contents → Focusing text widget (except on click) should select widget contents [also form <input> field]
Assignee | ||
Comment 116•22 years ago
|
||
Neil, your patch seems to work really well, for both HTML and XUL.
* It doesn't select text in text area, which is good.
* It selects text in textfields and XUL editable combo boxes, which is good.
* It doesn't select text on mouse click to focus, which is good.
I can't find anything wrong it -- however, I once heard hyatt say that the
htmlbindings files shouldn't have any javascript in them -- they should only
have command="foo". He said it would cause performance problems.
Hyatt, could you elucidate?
Comment 117•22 years ago
|
||
You can't add <implementation>s to common input fields without messing up the
prototype chain in JS. I've taken a policy of avoiding using <implementation>
with any standard HTML elements in order to keep their prototype chains
unaffected.
Comment 118•22 years ago
|
||
Another problem with my patch and Pete Zha's patch: focus events caused by
window activate or (over-eager?) use of .focus() also select all. So it looks as
if we still need bug 83496 fixed first.
Comment 119•22 years ago
|
||
Can we leverage MoveCaretToFocus to do this?
Assignee | ||
Comment 120•22 years ago
|
||
This gets everything correct that I know has been mentioned:
* It doesnt' select textareas
* It does select text in editable XUL comboboxes, such as used in the form
manager
* It doesn't select text from mouse clicks.
* It takes affect when a script focuses the textfield, or the user presses a
key (tab or accesskey) to get there.
* It selects text without putting it in the clipboard (because nsAutoCopy.cpp's
selection listener gets reason == nsISelectionListener::NO_REASON, and exits
early)
* It does the right thing for each platform. I used nsILookAndFeel for this,
instead of prefs because gtk2. I filed bug 172203 for this.
Attachment #74933 -
Attachment is obsolete: true
Attachment #96422 -
Attachment is obsolete: true
Assignee | ||
Comment 121•22 years ago
|
||
Seeking r=/sr=
Neil, thanks for the idea to leverege MoveCaretToFocus -- that worked great.
Assignee | ||
Comment 123•22 years ago
|
||
Still seeking r=/sr=
Attachment #101443 -
Attachment is obsolete: true
Comment 124•22 years ago
|
||
Comment on attachment 101484 [details] [diff] [review]
Same thing, but with one more null check on mCurrentFocus
>+ // First, select text fields when focused via keyboard (tab or accesskey)
>+ if (sTextfieldSelectModel == eTextfieldSelect_auto &&
>+ mCurrentFocus &&
>+ mCurrentFocus->IsContentOfType(nsIContent::eHTML_FORM_CONTROL)) {
>+ nsCOMPtr<nsIFormControl> formControl(do_QueryInterface(mCurrentFocus));
>+ PRInt32 controlType;
>+ formControl->GetType(&controlType);
>+ if (controlType == NS_FORM_INPUT_TEXT ||
>+ controlType == NS_FORM_INPUT_PASSWORD) {
>+ nsCOMPtr<nsIDOMHTMLInputElement> inputElement =
>+ do_QueryInterface(mCurrentFocus);
>+ if (inputElement) {
>+ inputElement->Select();
>+ }
>+ }
>+ }
>+
I'm not a C++ guru so I was just wondering whether you're being careful or you
can't just try QI to nsIDOMHTMLInputElement and call select regardless?
Assignee | ||
Comment 125•22 years ago
|
||
Neil, that would work -- but this is more performant, because QI's are more
expensive than a getter. In most cases the QI will fail, and a failing QI can be
somewhat expensive.
Comment 126•22 years ago
|
||
Comment on attachment 101484 [details] [diff] [review]
Same thing, but with one more null check on mCurrentFocus
r=brade but please switch Mac and Cocoa to do the selecting also
Attachment #101484 -
Flags: review+
Assignee | ||
Comment 128•22 years ago
|
||
Comment on attachment 101562 [details] [diff] [review]
New patch with brade's changes
Carrying r= forward
Attachment #101562 -
Flags: review+
Assignee | ||
Comment 129•22 years ago
|
||
Attachment #101562 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #101564 -
Flags: review+
Comment 130•22 years ago
|
||
Please comment in the various nsLookAndFeel impls on what the mysterious 1 and 0
values mean, referring the reader to the nsTextfieldSelectModel enum in
nsEventStateManager.h.
Assignee | ||
Comment 131•22 years ago
|
||
Attachment #101564 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #101567 -
Flags: review+
Assignee | ||
Comment 132•22 years ago
|
||
Attachment #101567 -
Attachment is obsolete: true
Assignee | ||
Comment 133•22 years ago
|
||
Attachment #101571 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Assignee | ||
Updated•22 years ago
|
Attachment #101573 -
Flags: review+
Comment 134•22 years ago
|
||
Comment on attachment 101573 [details] [diff] [review]
Better comments
sr=sfraser
Attachment #101573 -
Flags: superreview+
Comment 135•22 years ago
|
||
Comment on attachment 101573 [details] [diff] [review]
Better comments
>+ nsCOMPtr<nsIFormControl> formControl(do_QueryInterface(mCurrentFocus));
>+ PRInt32 controlType;
>+ formControl->GetType(&controlType);
>+ if (controlType == NS_FORM_INPUT_TEXT ||
>+ controlType == NS_FORM_INPUT_PASSWORD) {
>+ nsCOMPtr<nsIDOMHTMLInputElement> inputElement =
>+ do_QueryInterface(mCurrentFocus);
>+ if (inputElement) {
>+ inputElement->Select();
>+ }
>+ }
I just thought that you could cut it down from 2 to 1 QIs, I don't understand
the benefit in the "extra" QI to nsIFormControl to check the control type.
Assignee | ||
Comment 136•22 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 137•22 years ago
|
||
This patch has been checked into the CHIMERA_M1_0_1_BRANCH also.
Comment 138•22 years ago
|
||
verified - tried a number of cases (current win2k nightly) and they all seem to
work as desired.
Status: RESOLVED → VERIFIED
Updated•15 years ago
|
Keywords: helpwanted
You need to log in
before you can comment on or make changes to this bug.
Description
•