Closed Bug 188910 Opened 22 years ago Closed 20 years ago

focus is in the searchbox of the bookmarks/history sidebar when opening a new window; should only be there when opening the sidebar

Categories

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

Product:
Firefox
Component:
Bookmarks & History
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox0.9

People

(Reporter: henrik.borgvall, Assigned: steffen.wilberg)

References

Details

(Keywords: regression, Whiteboard: fixed0.9,fixed-aviary1.0)

Attachments

(1 file, 5 obsolete files)

unbitrotted again
3.92 KB, patch
mconnor
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20030112 Phoenix/0.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3a) Gecko/20030112 Phoenix/0.5

When the bookmark sidebar is opened, the focus is set to the content area rather
than the sidebar. Compare this behavior to the history sidebar, where focus is
set to the first item in the history (the today folder). In the bookmark
sidebar, I think focus should be set to the find field instead. This allows you
to quickly search for a bookmark using ctrl+b.

Reproducible: Always

Steps to Reproduce:
1. Start phoenix
2. Press CTRL+B


Actual Results:  
Bookmark sidebar appears. Focus is set to the content area.

Expected Results:  
Focus should be set to the bookmark sidebar. Preferably focus should be set to
the  find field in the sidebar.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Severity: normal → minor
Yep, I agree. Ctrl+B and start typing would be a big usability win.
Assignee: blaker → chanial
Component: General → Bookmarks
Target Milestone: --- → Phoenix0.6
Target Milestone: Phoenix0.6 → Phoenix0.8
taking QA contact, sorry about the bugspam
QA Contact: asa → mconnor
Attached patch patch (obsolete) — — Splinter Review 
A one-liner: document.getElementById("search-box").focus();
The rest is whitespace cleanup.
Comment on attachment 135848 [details] [diff] [review]
patch

Pierre, review?
Attachment #135848 - Flags: review?(p_ch)
-> All/all; taking.
Assignee: p_ch → steffen.wilberg
OS: Windows 2000 → All
Hardware: PC → All
While I'm at it, do the same for the history sidebar. This should really be
consistent with the bookmarks sidebar behaviour.
Summary: Focus should be set to search field when opening bookmark sidebar → Focus should be set to search field when opening bookmarks/history sidebar
Comment on attachment 135851 [details] [diff] [review]
do the same for the history sidebar

The very same line.
Attachment #135851 - Flags: review?(p_ch)
Found another minor focus problem with the bookmarks/history sidebar:

If you type something in the search field then press tab, it would be nice if
the first item found is selected. Now you have to press arrow down first.

Maybe it should be a new bug report?

What I wanted to do from the start was: CTRL-b - type something - tab - enter -
bookmark opens in the browser
Henrik: Please file a new bug. I don't know how to fix that right now, so let's
keep things simple here.
Comment on attachment 135848 [details] [diff] [review]
patch

If you have the sidebar visible and open a new window, the focus is in the
sidebar searchbox. I'm not sure if we want this.
steffen: we definitely do not want this to happen when opening a new window.

is it possible to modify the function that opens the sidebar to focus the 
correct element from that caller, instead of from within the document itself?  
it should be...
Comment on attachment 135848 [details] [diff] [review]
patch

I'll look into this. This patch is obsolete.
Attachment #135848 - Attachment is obsolete: true
Attachment #135848 - Flags: review?(p_ch)
Comment on attachment 135851 [details] [diff] [review]
do the same for the history sidebar

And this.
Attachment #135851 - Attachment is obsolete: true
Attachment #135851 - Flags: review?(p_ch)
Comment on attachment 135849 [details] [diff] [review]
same patch whithout whitespace cleanup (diff -u -w)

This one as well. I'll make a combined patch next time.
Attachment #135849 - Attachment is obsolete: true
Attached patch patch: modifies toggleSidebar() (obsolete) — — Splinter Review 
This is slightly more complex than the previous approach. :-)
It modifies toggleSidebar() in browser.js. My code is called when you open a
sidebar, not when you close it or open a new window.

The problem is, if the sidebar was hidden, the searchbox is not yet
constructed, and if another sidebar was displayed before, i.e. you're switching
sidebars, the searchbox of the previous sidebar is still displayed. In the
first case, there's nothing to focus; in the second, we don't want to focus the
wrong box.
So we have to check that and add an onload handler, which focusses the
searchbox after the (new) sidebar is constructed.
Comment on attachment 137401 [details] [diff] [review]
patch: modifies toggleSidebar()

Pierre, please have a look, especially at the eventListener stuff, since I have
no experience with that.
But it works fine.
Attachment #137401 - Flags: review?(p_ch)
-> 0.9
Target Milestone: Firebird0.8 → Firebird0.9
Comment on attachment 137401 [details] [diff] [review]
patch: modifies toggleSidebar()

bitrotted.
Attachment #137401 - Attachment is obsolete: true
Attachment #137401 - Flags: review?(p_ch)
Attached patch like before, unbitrotted (obsolete) — — Splinter Review 

    
  

        Attachment #140927 -
        Flags: review?(p_ch)

    
    

    
  


  

        Attachment #140927 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 141923 [details] [diff] [review]
unbitrotted again

Blake, after your checkin for bug 205107 (focus the search box in the history
sidebar) and the same in the bookmarks sidebar, the search box now gets
focussed upon opening a new window. I don't think we want that. It should
receive the focus upon opening the respective sidebar only. This patch does
that by attaching an onload handler to toggleSidebar. Also see comment 16.

        Attachment #141923 -
        Flags: review?(firefox)

    
  

        Attachment #140927 -
        Flags: review?(p_ch)

    
  
Severity: minor → normal
Summary: Focus should be set to search field when opening bookmarks/history sidebar → focus is in the searchbox of the bookmarks/history sidebar when opening a new window; should only be there when opening the sidebar

    
  
Whiteboard: new bug description in comment 22

    
    

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

    
    

    
  
This bug is in its current state (see comment 22) a regression from Blake's
checkins on 02/21:
http://bonsai.mozilla.org/cvsquery.cgi?date=explicit&mindate=02%2F21%2F2004+03%3A11&maxdate=02%2F21%2F2004+03%3A37
->adding regression keyword.
Keywords: regression
Flags: blocking0.9+
Priority: -- → P3

    
    

    
  
Comment on attachment 141923 [details] [diff] [review]
unbitrotted again

r=mconnor@myrealbox.com

        Attachment #141923 -
        Flags: review?(firefox) → review+

    
    

    
  
checked in on aviary branch, still needs trunk checkin, but weird things are
happening there, will follow up tomorrow when someone's around.
Whiteboard: new bug description in comment 22 → fixed0.9

    
    

    
  
fixed on trunk.  My trunk checkin tree was all busted up, nothing rm -rf *
couldn't fix though :)
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED

    
  
Whiteboard: fixed0.9 → fixed0.9,fixed-aviary1.0

    
    

    
  
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: