The default bug view has changed. See this FAQ.

search sidebar steals dialog focus.

RESOLVED FIXED in mozilla1.0.1

Status

SeaMonkey
Search
RESOLVED FIXED
16 years ago
3 years ago

People

(Reporter: andreww, Assigned: Brian Ryner (not reading))

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.0.1
x86
Other
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

16 years ago
windows build 20011022605

Steps to repeat:

start mozilla

make sure that the search sidebar is "topmost"

quit mozilla

launch mozilla

immediately hit ctrl+l to open an "open location" dialog.  You must do this
quickly before the search sidebar fully loads.

do nothing until the search sidebar finishes loading.

The search sidebar will steal cursor focus from the open location dialog and you
will be unable to type in that dialog unless you click again into the dialog
field.  

This may seem sorta fringe, but I've had this happen to me 3 times now in the
past few days and I think it falls into the "polish" and correctness category...

Comment 1

16 years ago
The search sidebar code was modified to focus its textbox as soon as it's 
created in bug 61417.  Due to bug 41813 and bug 50665 (recently marked as dups 
of bug 28467, I'm not sure why), this focus change isn't local to the search 
sidebar but instead grabs focus from any other part of the Mozilla window or 
even another Mozilla window.

The dialog problem will stick around even after the textbox.focus() problem is 
fixed, though, because it's likely that there will still be a way to steal 
focus from other windows (perhaps window.focus()).

Comment 2

16 years ago
I filed bug 76621 for the problem with sidebars stealing focus, so the 
remaining problem here is that the parent window of a modal dialog can steal 
focus from the dialog.

Comment 3

16 years ago
this is very annoying.  I never even use search in my sidebar, so it's about to
get unselected in my sidebar, but every time I open a new browser window and
search sidebar is open, it grabs focus, so I can't use space to page down in the
main window, or type in the url bar to go to a new site.  This is in 2001091203
on win98.

do we have a target milestone for this or is this dependent on some other bug?

Comment 4

16 years ago
Granrose, that's covered by bug 76621.  This bug is about things outside of 
dialogs being able to steal focus from dialogs.

Updated

16 years ago
Blocks: 89148

Comment 5

16 years ago
bryner,
Could you have look at this?
-- sidebar triage team
Assignee: matt → bryner

Comment 6

16 years ago
Created attachment 52246 [details] [diff] [review]
focus patch

Comment 7

16 years ago
last attachment was by matt@netscape.com on Cathleens machine.
(Assignee)

Comment 8

16 years ago
Would a better way to fix this be to check whether the sidebar tab is the active
window, and then focus the textbox if it is?  This way you don't lose the
auto-focus, but it won't steal focus either.  This is currently being done in
navigator.js for the content/urlbar focusing.... cc'ing jag to see if he has any
comments on that.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.7

Comment 9

16 years ago
That sounds reasonable. The code would indeed be very similar to the code that
currently prevents us from stealing focus with backgrounded windows:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=navigator.js&root=/cvsroot&subdir=mozilla/xpfe/browser/resources/content&command=DIFF_FRAMESET&rev1=1.373&rev2=1.374

Comment 10

16 years ago
Setting focus within a window (or within a content area) should be one command, 
called focus().  Setting focus should not require having separate cases for 
when the window has focus and when it doesn't, and it certainly shouldn't 
require having enough privileges to access nsIWindowWatcher.  See bug 77675, 
bug 97067, bug 76621, and bug 41813 for some of the problems caused by the 
current way of handling focus.

Comment 11

16 years ago
The problem is that not all platforms provide a |focus()| that doesn't activate
the window, so a similar logic (check whether the element's window is active and
if so just call focus, otherwise set focusedElement or focusedWindow) would need
to be put in all |focus()| implementations.

And then of course one would want a way to explicitely raise/activate a window
(or do we have that yet?)

Comment 12

16 years ago
>The problem is that not all platforms provide a |focus()| that doesn't activate
>the window, so a similar logic ... would need to be put in all |focus()| 
>implementations.

I'd rather that logic reside in the implementation of focus() rather than at 
each point where code needs to move focus within a window.

>And then of course one would want a way to explicitely raise/activate a window
>(or do we have that yet?)

I think most pop-under ads use window.focus() to bring the main page back to 
the front.  (In other words, some sites use window.focus() to hide the fact 
that the site is littering your desktop with ads.)  I'm not sure what you're 
supposed to use for "focus this frame" or "focus the content area, rather than 
the sidebar or the location bar" or "focus the content area rather than this 
text field, so the user can scroll the page with the arrow keys".
(Assignee)

Comment 13

16 years ago
This logic already exists for HTML text fields, see:
http://lxr.mozilla.org/seamonkey/source/content/html/content/src/nsHTMLInputElement.cpp#713

Perhaps we should just duplicate this for xul text fields?
(Assignee)

Comment 14

16 years ago
Not gonna make it for 0.9.7.
Target Milestone: mozilla0.9.7 → mozilla1.0

Comment 15

15 years ago
This affects chatzilla users with default channels too.  They start chatzilla,
which takes a few minutes to connect to the irc server and various initial
channels.  As each initial channel is successfully joined, the chatzilla input
box is focused, which of course, steals focus from whatever the user had been doing.

Comment 16

15 years ago
Claudius is search QA.

Also Samir, how come this is isn't assigned to you?
QA Contact: sujay → claudius

Comment 17

15 years ago
isn't the xul text field using the html text field?
QA Contact: claudius → sujay

Comment 18

15 years ago
Created attachment 64577 [details] [diff] [review]
patch -- overide focus() method to use html:input focus() method

This should do the trick. Not sure if there are any bad things that might be
going on ;-). I've tested it with chatzilla, works like a charm.

Updated

15 years ago
Keywords: mozilla0.9.8, patch
I used this patch, which is syntactically equivalent to basic's patch:

--- xpfe/global/resources/content/bindings/textbox.xml.bak	Wed Dec 19 03:58:08 2001
+++ xpfe/global/resources/content/bindings/textbox.xml	Sat Jan 12 12:01:18 2002
@@ -55,7 +55,7 @@
           this.inputField.select();
         </body>
       </method>
-
+      <method name="focus" action="this.inputField.focus();"/>
       <property name="controllers"    readonly="true" onget="return
this.inputField.controllers"/>
       <property name="textLength"     readonly="true" onget="return
this.inputField.textLength;"/>
       <property name="selectionStart" onset="this.inputField.selectionStart =
val; return val;"

Started Mozilla again (set to start mail/news).  Right around when the prompt
comes up to ask for my password, CRASH:  TB1571231Y, TB1570833H, TB1570821X.

Comment 20

15 years ago
Created attachment 64686 [details] [diff] [review]
patch -- overide focus() method to use html:input focus() method

sorry... that was bad. This should work.
Attachment #64577 - Attachment is obsolete: true
That time I just lost the textbox I was supposed to type my password into.  The
prompt came up fine, but there wasn't anything I could type into.

Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.7+) Gecko/20020110

Comment 22

15 years ago
Created attachment 64687 [details] [diff] [review]
patch -- overide focus() method to use html:input focus() method

patch -- overide focus() method to use html:input focus() method

lets do this again, it better work this time or Alex will kill me.
Attachment #64686 - Attachment is obsolete: true
This time, I had no major regressions.  Repeating the reporter's testcase, I
observed the search panel has a blinking caret, and the dialog box had a
blinking caret and focus.  Chatzilla also refused for the first time to take
focus back when I switched to browse or look at my JS console.  Very nice.

Guess I might as well hang up on the coroner...

Comment 24

15 years ago
I've been using moz with the last patch without any problems.
Brian: can you take a look at the patch?

Comment 25

15 years ago
--> search/claudius
Component: Sidebar → Search
QA Contact: sujay → claudius

Comment 26

15 years ago
note that the patch I attached has nothing to do with sidebar as it changes the
xul <textbox> to use the <html:input>'s focus() method when it's focus() method
is called.
(Assignee)

Comment 27

15 years ago
Comment on attachment 64687 [details] [diff] [review]
patch -- overide focus() method to use html:input focus() method

r=bryner, but long-term we should re-examine the way that we want focus and
activation to work.
Attachment #64687 - Attachment description: patch -- overide focus() method to use html:input focus() method → patch -- overide focus() method to use html:input focus() method
Attachment #64687 - Flags: review+

Comment 28

15 years ago
This is tricky, IMO too tricky to consider for 0.9.8.  There will need to be a 
lot of testing of this patch before it should go in to make sure there aren't 
regressions from it.  (Textfield focus in XUL is very tricky, i.e., very hacky.)

Comment 29

15 years ago
I thought that the tricky parts were handled by <html:input>, oh well guess I'll
have to suffer with chatzilla stealing focus every time I start/restart moz

Updated

15 years ago
Keywords: mozilla0.9.9
(Assignee)

Comment 30

15 years ago
This is an edge case, moving out...

(claudius or sairuh, would you have time to help with regression testing on this
patch?)
Target Milestone: mozilla1.0 → mozilla1.0.1

Comment 31

15 years ago
I don't think it's an edge case to leave the search sidebar panel open.

Updated

15 years ago
Blocks: 140346

Updated

15 years ago
Keywords: mozilla0.9.8, mozilla0.9.9

Updated

15 years ago
Keywords: mozilla1.0.1, mozilla1.1, review
(Assignee)

Comment 32

15 years ago
I think this will have been fixed by my checkin for bug 76621.  Can someone
please test on a current trunk or branch build and close this if it is fixed?
(Assignee)

Comment 33

15 years ago
yes, this is now fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.