Opening a new window with Cmd-K does not always focus the search field

VERIFIED FIXED in Firefox 6

Status

()

Firefox
Search
--
minor
VERIFIED FIXED
9 years ago
6 years ago

People

(Reporter: Marco De Vitis, Assigned: heycam)

Tracking

unspecified
Firefox 6
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; it; rv:1.9) Gecko/2008051202 Firefox/3.0
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; it; rv:1.9) Gecko/2008051202 Firefox/3.0

When all windows are closed and you press Cmd-K, a new window is opened and it should have the search field (in the toolbar) selected, ready for a search. This indeed happens sometimes, but very often the location field is selected instead. This behaviour appears to be random.

Reproducible: Sometimes

Steps to Reproduce:
1. Launch Firefox.
2. Close all Firefox windows.
3. Press Cmd-K.
Actual Results:  
A new window opens, and sometimes the cursor is inside the location bar.

Expected Results:  
The cursor should ALWAYS be placed in the search bar when opening a new window with Cmd-K.

After the window has opened, anyway, you can of course press Cmd-K again to move to the search field, if it wasn't initially selected.

Comment 1

9 years ago
It looks like the search bar is focused initially but then the address bar takes focus a few tenths of a second later.

Comment 2

9 years ago
This has apparently happened before, in bug 343903.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 3

9 years ago
I'm experiencing this as well.

Updated

9 years ago
Summary: Opening a new window with Cmd-K does not always activate the search field → Opening a new window with Cmd-K does not always focus the search field

Updated

9 years ago
Duplicate of this bug: 447959
(Assignee)

Comment 5

7 years ago
Created attachment 500758 [details] [diff] [review]
Make Command+K successfully focus the search box when it creates a new window.
Attachment #500758 - Flags: review?(gavin.sharp)
(Assignee)

Updated

7 years ago
Assignee: nobody → cam
Status: NEW → ASSIGNED
(Assignee)

Comment 6

6 years ago
Created attachment 525969 [details] [diff] [review]
Make Command+K successfully focus the search box when it creates a new window (v2)

Updated patch in accordance with some comments on IRC from Gavin.  This avoids racing for the window load event.
Attachment #500758 - Attachment is obsolete: true
Attachment #525969 - Flags: review?
Attachment #500758 - Flags: review?(gavin.sharp)
(Assignee)

Updated

6 years ago
Attachment #525969 - Flags: review? → review?(gavin.sharp)
You can pass a JS function object directly to addObserver (no need for the object/QI junk), and you can use arguments.callee for the removeObserver call.
(Assignee)

Comment 8

6 years ago
Created attachment 526163 [details] [diff] [review]
Make Command+K successfully focus the search box when it creates a new window (v3)

Removed QueryInterface method from the observer object.  I gave the observer a name though, just because it ended up looking neater than passing in the function directly to addObserver.

When do JS objects implementing interfaces need an explicit QueryInterface method and when not?
Attachment #525969 - Attachment is obsolete: true
Attachment #526163 - Flags: review?(gavin.sharp)
Attachment #525969 - Flags: review?(gavin.sharp)

Updated

6 years ago
Attachment #526163 - Attachment is patch: true
Attachment #526163 - Attachment mime type: application/octet-stream → text/plain
(In reply to comment #8)
> When do JS objects implementing interfaces need an explicit QueryInterface
> method and when not?

They generally never do, in the common cases (handler/observer JS object implementing a single interface being passed to a native C++ method). In this particular case, where the interface being implemented (nsIObserver) is marked "function" (http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIObserver.idl#45), you can just pass a JS function object directly.

I think you really only ever need the explicit QI for JS-implemented XPCOM components.
Comment on attachment 526163 [details] [diff] [review]
Make Command+K successfully focus the search box when it creates a new window (v3)

nit: use |function observer(...) {}| rather than |observer = function (...) {}| - that way the name is associated with the function object itself (rather than just being a reference to an anonymous function).

r=me with that change, thanks!
Attachment #526163 - Flags: review?(gavin.sharp) → review+

Comment 11

6 years ago
(In reply to comment #7)
> you can use arguments.callee for the removeObserver call
If there's plans on moving to strict mode in the future, arguments.callee won't work (or maybe just give a warning?).
|arguments.callee| flat-out won't work in strict mode code -- trying to access it will throw a TypeError.  Instead, any place where you'd use |arguments.callee|, just name the function, and use that name.  The only moderately reasonable excuse for ever using |arguments.callee| is to avoid IE's broken handling of function-name-scoping for such things (when you have |var a = function f(){}| it wrongly introduces an |f| binding as well as an |a| binding), but that reason's clearly inapplicable in Mozilla browser code.
OS: Mac OS X → Windows 7
(In reply to comment #12)
> Instead, any place where you'd use |arguments.callee|, just name the function,
> and use that name.

That's not suitable for e.g.:
foo.addEventListener("click", this.handler.bind(this), false);

or any other cases where you can't directly reference the attached function by name.
Recursive bound functions don't work as you think they work -- they invoke the target function in a nested call, so the effect is as if the original function were called (tho I'm not sure about the effect on operations that actually try to walk the stack, rather than just look at the immediate frame):

[jwalden@find-waldo-now src]$ dbg/js
js> function f() { return arguments.callee === f; }
js> f.bind(null)()
true

So arguments.callee within a function that's being called from a bound function is that function, not the bound function.

If you have other cases "where you can't directly reference the attached function by name", I'd be interested to hear them.  I'm not aware of other cases, myself.
OS: Windows 7 → Mac OS X
(In reply to comment #14)
> So arguments.callee within a function that's being called from a bound function
> is that function, not the bound function.

That's unfortunate... Having to keep event listener functions referencable is annoying.
Cameron, are you going to land this?
(Assignee)

Comment 17

6 years ago
I forgot, I did land it!

http://hg.mozilla.org/mozilla-central/rev/37ad2e03e38e

Is that still within the "Target Milestone: firefox5" or is it firefox6?  Also, would I determine the answer to that?

I got a drive-by comment earlier to swap the order of the function statement and then window.openDialog() call, because it may be confusing to see the `if (subject == win)` check in the branch where we test that win is falsey.  (Of course it doesn't actually matter, since function statements are hoisted to the top of the containing function, and the `win` is evaluated at the time of the function call, when it's not falsey.)  I wanted to check whether you think it is worth making this change for de-confusion.

Comment 18

6 years ago
http://hg.mozilla.org/mozilla-central/tags says AURORA_BASE_20110412 is mozilla-central changeset a95d42642281.

The patch in this bug landed as rev mozilla-central changeset 37ad2e03e38e.

"hg debugancestor 37ad2e03e38e a95d42642281" says a95d42642281 (the branch point) is the earlier revision. So this landed for Firefox 6, not for Firefox 5.
(Assignee)

Comment 19

6 years ago
Thanks.
Target Milestone: --- → Firefox 6
Does it mean we can close out the bug as fixed now?
yes
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 22

6 years ago
Setting resolution to Verified Fixed on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1) Gecko/20110628 Firefox/7.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.