90587 - clickSelectsAll should not trigger on task switch if textbox already had focus (url bar selected unnecessarily when switching windows while editing)

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P4)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Click on the location bar.  (Result: URL is selected)
2. Click on the location bar again.  (Result: location bar is focused but not
selected)
3. Switch to another app and back using the mouse or Alt+Tab.

Result: the entire URL is selected.
Expected: insertion point or selection should be the same as when I left the window.

IE also has this bug, and I run into it several times a week while using IE. 
(Blake: you did an amazingly accurate job of copying IE's bugs when you helped
us copy this feature.)

Suggested fix: use onclick instead of onfocus for clickSelectsAll.  This will
break the case of tabbing to the location bar until bug 28583, "Focusing text
widget (except on click) should select widget", is fixed, but won't affect Ctrl+L.

Comment 1

[timeless]

hrm. this also happens to the run dialog w/ xmouse. I'm not sure i consider 
this a bug.  nc4's behavior is to set focus to _content which is annoying.

AIM signon behaves like run.  This appears to be the win32 standard behavior.

steps: run aim w/ no network or MyAim>switch screen name.
click in the middle of your screen name. change apps.

a bit more info from terminal services client. if the box is a listbox you'll 
select all, if it's an editbox you won't.  the urlbar is clearly a listbox.

AIM of course is broken, if you try leaving focus in the password field it will 
wonder to the screenname field (select all) -- anyone want to file a (ns 
internal) bug for that ;-?

Comment 2

[timeless]

after thinking about it i guess we should rename it to focusSelectsAll since 
that's what the behavior really is and should be ...

Comment 3

[Matthew T (active 1999-2002)]

No, focusSelectsAll is bug 28583 and should apply to all text fields, not just 
the address field.

Comment 4

[Alec Flett]

reassign url bar bugs to new owner..

Comment 5

[Blake Ross]

Joe was dying to have these bugs. Who am I to say no?

Comment 6

[Jesse Ruderman]

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

Comment 7

[Jesse Ruderman]

[e-mail] said on [e-mail] that this breaks
NaturallySpeaking 6.1, which temporarily takes focus from applications.  "If I
dictate a URL and try to correct a word, when focus returns to the URL window,
the entire URL is selected and the correction overwrites what I have previously
spoken."

Comment 8

[Chris Tubutis]

Not sure what the status of the bug listed here is or if I'm in the right place
but I have a comment. Firebird 0.7 on Linux, single-clicking the location bar
selects EVERYTHING. The entire URL. I HATE HATE HATE that, it is NOT proper UNIX
behavior, that is Win32 behavior. I ought to be able to single-click anywhere in
the location bar to place the I-beam where I want it so I can edit things;
double-clicking should select everything.

Comment 9

[Jesse Ruderman]

Chris: you're not in the right place.

Comment 10

[Nikitas Liogkas]

I believe the simplest fix for this is to remove the 'onfocus' handler from the
'urlbar' element in browser.xul. As far as I can tell, this gets rid of the bug,
but doesn't affect the rest of the location bar behavior.

Nikitas Liogkas

Comment 11

[Chris Wood]

WORKAROUND: The Autocomplete Manager extension (https://addons.mozilla.org/extensions/moreinfo.php?id=2300) includes a bugfix for this.

Comment 13

[Dão Gottwald [:dao]]

Attachment: patch

Comment 14

[Neil Deakin]

Comment on attachment 375813 [details] [diff] [review]
patch

>+          // don't trigger clickSelectsAll when switching application windows
>+          if (document.activeElement == this.inputField)

This is a fragile check and it won't work at all after bug 178324 is done. What you actually want is a means of knowing if the element is within a toplevel window that is no longer active. I'll think about the best way to do this a bit.

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 375813 [details] [diff] [review]
patch

this still seems to work, despite bug 178324

Comment 17

[Neil Deakin]

What I mean is that if your goal is to check only when the entire window is switched, the check here isn't sufficient. For instance, if the textbox is in a frame, document.activeElement will still be set to the textbox when switching to an element in another frame. Another example would be a textbox displayed in a tab. Switching tabs would not change the value of document.activeElement.

I'm not saying the check here is incorrect, but I want to ensure that you are checking the right thing.

Comment 18

[Dão Gottwald [:dao]]

Attachment: patch v2

Alright, so this won't fix it for frames and content windows.

Comment 19

[Neil Deakin]

Comment on attachment 383789 [details] [diff] [review]
patch v2

>+              window.constructor == ChromeWindow

Isn't just using 'instanceof' sufficient here?

Comment 20

[Dão Gottwald [:dao]]

(In reply to comment #19)
> (From update of attachment 383789 [details] [diff] [review])
> >+              window.constructor == ChromeWindow
> 
> Isn't just using 'instanceof' sufficient here?

For some reason, this gives me 'true':
javascript:window%20instanceof%20ChromeWindow

Comment 21

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/914a46ad38b6