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

VERIFIED FIXED in mozilla1.0

Status

()

Core
XUL
P3
normal
VERIFIED FIXED
18 years ago
8 years ago

People

(Reporter: Matthew Paul Thomas, Assigned: Aaron Leventhal)

Tracking

({access, polish})

Trunk
mozilla1.0
access, polish
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [behavior])

Attachments

(1 attachment, 13 obsolete attachments)

12.33 KB, patch
Aaron Leventhal
: review+
Simon Fraser
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

18 years ago
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.
(Reporter)

Updated

18 years ago
Keywords: 4xp

Comment 1

18 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

Comment 2

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

Comment 3

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

Comment 4

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

Comment 5

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

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M20

Comment 6

18 years ago
Minor usability issue, scheduling for M20!

Updated

18 years ago
QA Contact: paulmac → sujay

Comment 7

17 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

17 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 9

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

Comment 10

17 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

17 years ago
Created attachment 8221 [details] [diff] [review]
rough patch

Comment 12

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

Comment 13

17 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 14

17 years ago
Reassigning to beppe (editor).
Assignee: buster → beppe

Comment 15

17 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

Comment 16

17 years ago
Accepting bug.
Status: NEW → ASSIGNED
(Reporter)

Comment 17

17 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

17 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

17 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

17 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.
Severity: enhancement → minor
Keywords: pp, rtm

Comment 21

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

Comment 22

17 years ago
adding help wanted keyword
Keywords: helpwanted

Updated

17 years ago
Keywords: 4xp, pp, rtm
(Reporter)

Comment 23

17 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
(Reporter)

Updated

17 years ago
Blocks: 37587

Comment 24

17 years ago
*** Bug 60610 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 25

17 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

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

Comment 27

17 years ago
*** Bug 74003 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

16 years ago
No longer blocks: 37587

Comment 28

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

Comment 29

16 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.
Keywords: nsbeta1, nsCatFood, pp
Target Milestone: Future → ---

Comment 30

16 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

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

Updated

16 years ago
Target Milestone: mozilla0.9.1 → mozilla0.9.2

Comment 32

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

Updated

16 years ago
Whiteboard: [behavior]

Updated

16 years ago
Depends on: 83496

Comment 33

16 years ago
11 dups. marking mostfreq.
Keywords: mostfreq

Comment 34

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

Comment 35

16 years ago
*** Bug 81377 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 36

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

Comment 37

16 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

16 years ago
*** Bug 89339 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 39

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

Comment 40

16 years ago
*** Bug 96652 has been marked as a duplicate of this bug. ***
*** Bug 62880 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Blocks: 104166

Updated

16 years ago
Blocks: 49574
(Reporter)

Comment 42

16 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

Updated

16 years ago
Blocks: 89689

Comment 43

16 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

16 years ago
nominating for nsbeta1, adding access keyword
Keywords: access, nsbeta1
Marking nsbeta1+

Updated

16 years ago
Target Milestone: mozilla1.0.1 → mozilla1.0
Rod, can you take a look at this?
Assignee: kin → rods
Status: ASSIGNED → NEW
Keywords: nsbeta1 → nsbeta1+
nsbeta1-. Engineers are overloaded with higher priority bugs. 
Keywords: nsbeta1+ → nsbeta1-

Comment 48

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

Updated

16 years ago
Depends on: 127272
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
Keywords: nsbeta1- → nsbeta1+
(Assignee)

Comment 50

16 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

16 years ago
"had moved onto something with it's own caret"

Can you explain?
(Assignee)

Comment 52

16 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

16 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

16 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)?
(Assignee)

Updated

16 years ago
Depends on: 127812

Updated

16 years ago
No longer depends on: 127812

Updated

16 years ago
Blocks: 127812

Comment 55

16 years ago
Created attachment 72569 [details] [diff] [review]
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

16 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

16 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

16 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

16 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

16 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

16 years ago
Created attachment 73178 [details] [diff] [review]
New Patch

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

16 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

16 years ago
removing myself from the cc list

Comment 64

16 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

16 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

16 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

16 years ago
Created attachment 73621 [details] [diff] [review]
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

16 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

16 years ago
Yes, Danmian, I think you are quite right.

Comment 70

16 years ago
Yes, Damian, I think you are quite right.

Comment 71

16 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

16 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

16 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

16 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

16 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

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

Comment 77

16 years ago
Mozilla does currently remember position and selection state when tabbing
through textboxes.
*** Bug 131185 has been marked as a duplicate of this bug. ***

Comment 79

16 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

16 years ago
Created attachment 74933 [details] [diff] [review]
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
Attachment #73178 - Attachment is obsolete: true
Attachment #73621 - Attachment is obsolete: true

Comment 81

16 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

16 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

16 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

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

Taking.
Assignee: rods → aaronl
(Assignee)

Comment 85

16 years ago
-> Pete Zha 

We discussed the correct fix on IRC.
Assignee: aaronl → pete.zha
(Reporter)

Comment 86

16 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

16 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

16 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

16 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

16 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

16 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

16 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

16 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

16 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

16 years ago
Kee: that's bug 116441.

Comment 96

16 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

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

Updated

16 years ago
Depends on: 90587
*** Bug 141869 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Keywords: polish
*** Bug 155683 has been marked as a duplicate of this bug. ***

Comment 100

15 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

15 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

15 years ago
*** Bug 163232 has been marked as a duplicate of this bug. ***
Created attachment 96297 [details] [diff] [review]
Basic XBL-based patch

Note that this only affects XUL textboxes, not editable menulists or HTML.
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

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

15 years ago
Can we please also fix this for editable menulist?

Comment 108

15 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.
Recent versions of gtk on linux do select all the text when you give focus to a
widget.
Do they also copy the text to the SELECTION buffer? (or whatever the buffer is
called that gets pasted on middle-click)

Comment 111

15 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

15 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.
> 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?
Created attachment 96422 [details] [diff] [review]
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.

Updated

15 years ago
Attachment #96297 - Attachment is obsolete: true
*** Bug 170576 has been marked as a duplicate of this bug. ***

Updated

15 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

15 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

15 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.
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.
Can we leverage MoveCaretToFocus to do this?
(Assignee)

Comment 120

15 years ago
Created attachment 101443 [details] [diff] [review]
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.
Attachment #74933 - Attachment is obsolete: true
Attachment #96422 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Depends on: 172203
(Assignee)

Comment 121

15 years ago
Seeking r=/sr=

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

Comment 122

15 years ago
taking
Assignee: pete.zha → aaronl
(Assignee)

Comment 123

15 years ago
Created attachment 101484 [details] [diff] [review]
Same thing, but with one more null check on mCurrentFocus

Still seeking r=/sr=
Attachment #101443 - Attachment is obsolete: true
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

15 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

15 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 127

15 years ago
Created attachment 101562 [details] [diff] [review]
New patch with brade's changes

Seeking sr=
Attachment #101484 - Attachment is obsolete: true
(Assignee)

Comment 128

15 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

15 years ago
Created attachment 101564 [details] [diff] [review]
Oops, forgot to change PRInt32 -> PRInt8 for static in both places
Attachment #101562 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #101564 - Flags: review+

Comment 130

15 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

15 years ago
Created attachment 101567 [details] [diff] [review]
Comments added to nsLookAndFeel.cpp, explaining what the mysterious values are
Attachment #101564 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #101567 - Flags: review+
(Assignee)

Comment 132

15 years ago
Created attachment 101571 [details] [diff] [review]
Better comments
Attachment #101567 - Attachment is obsolete: true
(Assignee)

Comment 133

15 years ago
Created attachment 101573 [details] [diff] [review]
Better comments
Attachment #101571 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Blocks: 172203
No longer depends on: 172203
(Assignee)

Updated

15 years ago
Attachment #101573 - Flags: review+

Comment 134

15 years ago
Comment on attachment 101573 [details] [diff] [review]
Better comments

sr=sfraser
Attachment #101573 - Flags: superreview+
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

15 years ago
checked in
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 137

15 years ago
This patch has been checked into the CHIMERA_M1_0_1_BRANCH also.

Comment 138

14 years ago
verified - tried a number of cases (current win2k nightly) and they all seem to
work as desired.
Status: RESOLVED → VERIFIED

Updated

8 years ago
Keywords: helpwanted
You need to log in before you can comment on or make changes to this bug.