385062 - Search field in bookmarks sidebar gets the focus when opening a new window

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

The search field in the bookmarks sidebar is taking the focus (as opposed to the location bar) when new windows are created.

Steps to reproduce:
1) Open bookmarks sidebar
2) Close window/close minefield
3) Open new window/open minefield
4) Start typing

Comment 1

[(not reading, please use [e-mail] instead)]

if you look at http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/content/bookmarksPanel.js#40, we focus the text box on init()

I was about to say that this was regression from fx 2, but looking at http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/content/bookmarksPanel.js, I'm not so sure.

alex:  do you think that opening up the bm sidebar should focus the text box, but that starting with the bm sidebar open should not?

note, the history side bar does not focus in either case.

Comment 2

[(not reading, please use [e-mail] instead)]

see also bug #322077

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

Yeah, if you open the bookmarks sidebar, the search box should get the focus, but if you open a new browser window that has the bookmarks sidebar open, the location bar should have the focus.

Comment 4

[(not reading, please use [e-mail] instead)]

alex:  same for the history sidebar, I assume?

also, what about when we open up a new window and a bookmark is loaded in the sidebar?

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

Out of curiosity, how to we determine which text field in the content area gets the focus when opening a new window?  (all of the above comments assume a home page of about:blank).

So your question is what do we do if you open a new window and a bookmark is loaded in the bookmark sidebar, and it contains a text field?  I would probably give the focus first to the content area, and then to the location bar, before given the bookmark sidebar bookmark focus.  Does this seem right? 

Comment 6

[(not reading, please use [e-mail] instead)]

> note, the history side bar does not focus in either case.

I misspoke.  the history sidebar has the same bug, see:

http://lxr.mozilla.org/seamonkey/source/browser/components/places/content/history-panel.js#69

Comment 7

[PikeUK]

Originally fixed in Bug 188910.

Comment 8

[Nickolay_Ponomarev]

I think we should put back the code I removed in bug 322077 (my bad, didn't see it was added intentionally in bug 188910), but instead of focusing the element directly from toggleSidebar, fire a 'sidebar-opened' event on the doc in the sidebar (after it is fully loaded) and let a script in that document handle the focusing if it needs to. Any objections?

Comment 9

[Nickolay_Ponomarev]

Attachment: patch

As described, this adds a SidebarFocused event fired on the |window| of the sidebar doc. The handlers in bookmark/history panels take care of focusing the search box.

I also made toggleSidebar(forceOpen=true) fire this event, because it made sense. This change doesn't affect built-in Firefox sidebars and doesn't affect old extension sidebars that don't make use of the new event.

The event is fired off a 0ms timeout from a capturing 'load' handler. This lets the (non-capturing) load handlers run in the sidebar. I don't know a better way to get notified after the doc is fully loaded and its load handlers are run.

Comment 10

[Nickolay_Ponomarev]

To properly test this I had to apply the patch from bug 406697.

And of course I didn't add a comment explaining why the new event is needed. What about this:
+ * Fire a "SidebarFocused" event on the sidebar's |window| to give the sidebar
+ * a chance to adjust focus as needed. An additional event is needed, because
+ * we don't want to focus the sidebar when it's opened on startup or in a new
+ * window, only when the user opens the sidebar.
+ */
+function fireSidebarFocusedEvent() {

?

Comment 11

[PikeUK]

Asking for blocking, because:
a) It's IMHO currently the most annoying bug in Fx3 if you leave your sidebar open permanently.
b) It has a patch.
c) It's a regression from Fx2.
d) The original bug (Bug 188910) was marked as blocking.

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

I would like blocking as well.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #12)
> I would like blocking as well.

(me-too comments aren't really needed, if you feel like the blocking nomination failed to adequately address all blocking rationale, then by all means add some)

Comment 14

[Nickolay_Ponomarev]

Thanks, Gavin!

If someone beats me to checking this in, please make the change from comment 10.

Comment 15

[Reed Loden [:reed]]

Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.1002; previous revision: 1.1001
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.1003; previous revision: 1.1002
done
Checking in browser/components/places/content/bookmarksPanel.js;
/cvsroot/mozilla/browser/components/places/content/bookmarksPanel.js,v  <--  bookmarksPanel.js
new revision: 1.19; previous revision: 1.18
done
Checking in browser/components/places/content/history-panel.js;
/cvsroot/mozilla/browser/components/places/content/history-panel.js,v  <--  history-panel.js
new revision: 1.38; previous revision: 1.37
done

Comment 16

[Tracy Walker [:tracy]]

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre

however, focus is in content, not the location bar. That is what happens with a browser window only new window.

Comment 17

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv