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.

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

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

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

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

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

Minor usability issue, scheduling for M20!

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.

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

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

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.

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

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.

Reassigning to beppe (editor).

assigning to Kin for review

Accepting bug.

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.

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)

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.

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.

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

adding help wanted keyword

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

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

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.

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

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

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

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.

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

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

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

11 dups. marking mostfreq.

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

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

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

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

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

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

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

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

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

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

nominating for nsbeta1, adding access keyword

Marking nsbeta1+

Rod, can you take a look at this?

nsbeta1-. Engineers are overloaded with higher priority bugs.

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.

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

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.

"had moved onto something with it's own caret" Can you explain?

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

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.

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

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

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

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

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.

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

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

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.

removing myself from the cc list

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?

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?

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

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

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.

Yes, Danmian, I think you are quite right.

Yes, Damian, I think you are quite right.

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.

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.

Hi, Does selection only apply to widget with localname as: text and input? If so, we can only select content with such names.

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.

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.

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

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

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

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.

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

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

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.

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

-> Pete Zha We discussed the correct fix on IRC.

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.

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)

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.

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

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.

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.

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.

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.

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.

Kee: that's bug 116441.

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.

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.

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

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

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

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.

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

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

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.

Can we please also fix this for editable menulist?

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)

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.

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

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.

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

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?

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?

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.

Seeking r=/sr= Neil, thanks for the idea to leverege MoveCaretToFocus -- that worked great.

taking

Still seeking r=/sr=

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?

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

Seeking sr=

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

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 on attachment 101573 [details] [diff] [review] Better comments sr=sfraser

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.

checked in

This patch has been checked into the CHIMERA_M1_0_1_BRANCH also.

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