28583 - Focusing text widget (except on click) should select widget contents [also form <input> field]

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Matthew T (active 1999-2002)]

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

[Peter Trudelle]

reproduced in recent verification build on Win98. reassigning to pollmann. Eric, 
is this part of the tab mgr work I hear you are doing?

Comment 2

[Peter Trudelle]

*** Bug 23276 has been marked as a duplicate of this bug. ***

Comment 3

[lchiang]

to qa: when this bug is fixed, pls notify esther or lchiang so we can try our 
dup bug case. Thanks.

Comment 4

[Peter Trudelle]

*** Bug 25458 has been marked as a duplicate of this bug. ***

Comment 5

[John Morrison]

*** Bug 30230 has been marked as a duplicate of this bug. ***

Comment 6

[Eric Pollmann]

Minor usability issue, scheduling for M20!

Comment 7

[Brendan Donohoe]

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

[Stanton McCandlish]

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 9

[Asa Dotzler [:asa]]

*** Bug 37769 has been marked as a duplicate of this bug. ***

Comment 10

[Eric Pollmann]

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.

Comment 11

[Eric Pollmann]

Attachment: rough patch

Comment 12

[Eric Pollmann]

*** Bug 36278 has been marked as a duplicate of this bug. ***

Comment 13

[Eric Pollmann]

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 14

[karnaze (gone)]

Reassigning to beppe (editor).

Comment 15

[rubydoo123]

assigning to Kin for review

Comment 16

[kinmoz]

Accepting bug.

Comment 17

[Matthew T (active 1999-2002)]

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

[rods (gone)]

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

[Decklin Foster]

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.

Comment 20

[Matthew T (active 1999-2002)]

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

[Decklin Foster]

*doublechecks*
Yes, GTK really does not do it. I probably would have complained already if it
did ;-)

Comment 22

[rubydoo123]

adding help wanted keyword

Comment 23

[Matthew T (active 1999-2002)]

Not an RFE --> removing `[RFE]' from summary. Given the number of times it 
happens in normal usage, not a minor issue either --> normal.

Comment 24

[Jesse Ruderman]

*** Bug 60610 has been marked as a duplicate of this bug. ***

Comment 25

[Matthew T (active 1999-2002)]

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.

Comment 26

[Matthew T (active 1999-2002)]

*** Bug 66284 has been marked as a duplicate of this bug. ***

Comment 27

[Zach Lipton [:zach]]

*** Bug 74003 has been marked as a duplicate of this bug. ***

Comment 28

[Kathleen :Brade]

*** Bug 69310 has been marked as a duplicate of this bug. ***

Comment 29

[Kathleen :Brade]

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

[kinmoz]

Set to mozilla0.9.1 for now. Cc'ing cmanske who had the dup of this bug.

Comment 31

[Simon Fraser [no longer active]]

We probably need an nsILookAndFeel entry for this. I so wish it would work on 
Mac.

Comment 32

[Kathleen :Brade]

*** Bug 79344 has been marked as a duplicate of this bug. ***

Comment 33

[Jacek Piskozub]

11 dups. marking mostfreq.

Comment 34

[Jesse Ruderman]

*** Bug 73078 has been marked as a duplicate of this bug. ***

Comment 35

[Jesse Ruderman]

*** Bug 81377 has been marked as a duplicate of this bug. ***

Comment 36

[Matthew T (active 1999-2002)]

*** Bug 83588 has been marked as a duplicate of this bug. ***

Comment 37

[rubydoo123]

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

Comment 38

[Jesse Ruderman]

*** Bug 89339 has been marked as a duplicate of this bug. ***

Comment 39

[Matthew T (active 1999-2002)]

*** Bug 62880 has been marked as a duplicate of this bug. ***

Comment 40

[Kathleen :Brade]

*** Bug 96652 has been marked as a duplicate of this bug. ***

Comment 41

[Boris Zbarsky [:bzbarsky]]

*** Bug 62880 has been marked as a duplicate of this bug. ***

Comment 42

[Matthew T (active 1999-2002)]

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

Comment 43

[kinmoz]

Bulk move of mozilla1.0 bugs to mozilla.1.0.1. I will try to pull some of these
back in if I can.

Comment 44

[Peter Trudelle]

nominating for nsbeta1, adding access keyword

Comment 45

[Kevin McCluskey (gone)]

Marking nsbeta1+

Comment 46

[Kevin McCluskey (gone)]

Rod, can you take a look at this?

Comment 47

[Kevin McCluskey (gone)]

nsbeta1-. Engineers are overloaded with higher priority bugs. 

Comment 48

[Peter Trudelle]

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

[Kevin McCluskey (gone)]

Kin and Rod think the editor changes should be easy to implement once bug 83496
is fixed.

Marking this bug nsbeta1+ and nominating bug 83496 for nsbeta1

Comment 50

[Aaron Leventhal]

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

[Simon Fraser [no longer active]]

"had moved onto something with it's own caret"

Can you explain?

Comment 52

[Aaron Leventhal]

textfields and textareas are editable, and as such have their own
caret/selection that is separate from the document's caret/selection.

Comment 53

[Kathleen :Brade]

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

[Simon Fraser [no longer active]]

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

[Jim Song]

Attachment: SelectAll When ShiftFocus

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

[Kathleen :Brade]

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

[e-mail]--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.

Comment 57

[Kathleen :Brade]

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

Comment 58

[Aaron Leventhal]

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

[Simon Fraser [no longer active]]

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

[Jim Song]

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

[Jim Song]

Attachment: New Patch

I have added code to show the cursor if the content is empty.
Please review.

Comment 62

[kinmoz]

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

[rubydoo123]

removing myself from the cc list

Comment 64

[Jim Song]

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

[Jim Song]

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?

Comment 66

[Aaron Leventhal]

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

[Jim Song]

Attachment: Patches with Checking Text Widgets

Add Aaron's codes to check if it's a text field.
A small change from Aaron's is:
if( frameSelection && ... )

Comment 68

[Damian Yerrick]

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

[Jim Song]

Yes, Danmian, I think you are quite right.

Comment 70

[Jim Song]

Yes, Damian, I think you are quite right.

Comment 71

[Peter Trudelle]

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

[Peter Lairo]

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

[Jim Song]

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

[timeless]

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.

Comment 75

[Aaron Leventhal]

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

[Simon Fraser [no longer active]]

I agree with timeless. We should be able to remember where the selection was in
a textarea.

Comment 77

[Samuel Sieb]

Mozilla does currently remember position and selection state when tabbing
through textboxes.

Comment 78

[Boris Zbarsky [:bzbarsky]]

*** Bug 131185 has been marked as a duplicate of this bug. ***

Comment 79

[Pete Zha]

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

[Pete Zha]

Attachment: Patch just add SelectAll in nsHTMLInputElement.cpp

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

Comment 81

[Axel Hecht]

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

[Kathleen :Brade]

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

Comment 83

[Charles Manske]

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.

Comment 84

[Aaron Leventhal]

I believe I have the right fix for this bug, by implementing bug 83496.

Taking.

Comment 85

[Aaron Leventhal]

-> Pete Zha 

We discussed the correct fix on IRC.

Comment 86

[Matthew T (active 1999-2002)]

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

[Axel Hecht]

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

[Roland Mainz]

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.

Comment 89

[Matthew T (active 1999-2002)]

> 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

[Axel Hecht]

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.

Comment 91

[Matthew T (active 1999-2002)]

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

[Akkana Peck]

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.

Comment 93

[Matthew T (active 1999-2002)]

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

[Kee Hinckley]

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

[Simon Fraser [no longer active]]

Kee: that's bug 116441.

Comment 96

[Akkana Peck]

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

[Val Henson]

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

[Boris Zbarsky [:bzbarsky]]

*** Bug 141869 has been marked as a duplicate of this bug. ***

Comment 99

[Boris Zbarsky [:bzbarsky]]

*** Bug 155683 has been marked as a duplicate of this bug. ***

Comment 100

[Luke Morey]

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

Comment 101

[Aaron Leventhal]

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

[Jesse Ruderman]

*** Bug 163232 has been marked as a duplicate of this bug. ***

Comment 103

[[e-mail]]

Attachment: Basic XBL-based patch

Note that this only affects XUL textboxes, not editable menulists or HTML.

Comment 104

[[e-mail]]

Comment on attachment 96297 [details] [diff] [review]
Basic XBL-based patch

[e-mail] 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

[Kathleen :Brade]

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

[[e-mail]]

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

[Charles Manske]

Can we please also fix this for editable menulist?

Comment 108

[Charles Manske]

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

[Christopher Blizzard (:blizzard)]

Recent versions of gtk on linux do select all the text when you give focus to a
widget.

Comment 110

[Christian :Biesinger (don't email me, ping me on IRC)]

Do they also copy the text to the SELECTION buffer? (or whatever the buffer is
called that gets pasted on middle-click)

Comment 111

[Akkana Peck]

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

[[e-mail]]

"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

[[e-mail]]

> 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

[[e-mail]]

Attachment: Proposed patch

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.

Comment 115

[Boris Zbarsky [:bzbarsky]]

*** Bug 170576 has been marked as a duplicate of this bug. ***

Comment 116

[Aaron Leventhal]

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

[David Hyatt]

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

[[e-mail]]

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

[[e-mail]]

Can we leverage MoveCaretToFocus to do this?

Comment 120

[Aaron Leventhal]

Attachment: Uses esm:MoveCaretToFocus to check for textfield/password controls and select them

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.

Comment 121

[Aaron Leventhal]

Seeking r=/sr=

Neil, thanks for the idea to leverege MoveCaretToFocus -- that worked great.

Comment 122

[Aaron Leventhal]

taking

Comment 123

[Aaron Leventhal]

Attachment: Same thing, but with one more null check on mCurrentFocus

Still seeking r=/sr=

Comment 124

[[e-mail]]

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?

Comment 125

[Aaron Leventhal]

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

[Kathleen :Brade]

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

Comment 127

[Aaron Leventhal]

Attachment: New patch with brade's changes

Seeking sr=

Comment 128

[Aaron Leventhal]

Comment on attachment 101562 [details] [diff] [review]
New patch with brade's changes

Carrying r= forward

Comment 129

[Aaron Leventhal]

Attachment: Oops, forgot to change PRInt32 -> PRInt8 for static in both places

Comment 130

[Simon Fraser [no longer active]]

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.

Comment 131

[Aaron Leventhal]

Attachment: Comments added to nsLookAndFeel.cpp, explaining what the mysterious values are

Comment 132

[Aaron Leventhal]

Attachment: Better comments

Comment 133

[Aaron Leventhal]

Attachment: Better comments

Comment 134

[Simon Fraser [no longer active]]

Comment on attachment 101573 [details] [diff] [review]
Better comments

sr=sfraser

Comment 135

[[e-mail]]

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.

Comment 136

[Aaron Leventhal]

checked in

Comment 137

[Simon Fraser [no longer active]]

This patch has been checked into the CHIMERA_M1_0_1_BRANCH also.

Comment 138

[Michael Lefevre]

verified - tried a number of cases (current win2k nightly) and they all seem to
work as desired.