Closed Bug 385062 Opened 14 years ago Closed 13 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 3 beta5

People

(Reporter: faaborg, Assigned: asqueella)

References

Details

(Keywords: regression)

Attachments

(1 file)

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
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.
OS: Mac OS X → All
Hardware: PC → All
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.

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?
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? 
> 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
Originally fixed in Bug 188910.
Keywords: regression
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?
Assignee: nobody → asqueella
Blocks: 322077
Attached patch patchSplinter Review
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.
Attachment #304442 - Flags: review?(gavin.sharp)
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() {

?
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.
Flags: blocking-firefox3?
I would like blocking as well.
(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)
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Whiteboard: [has patch][needs review gavin]
Attachment #304442 - Flags: review?(gavin.sharp) → review+
Status: NEW → ASSIGNED
Whiteboard: [has patch][needs review gavin] → [has patch]
Target Milestone: --- → Firefox 3 beta5
Thanks, Gavin!

If someone beats me to checking this in, please make the change from comment 10.
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch]
Depends on: 423392
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.
Status: RESOLVED → VERIFIED
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.