50223 - Clicking on Sidebar tab should set focus to sidebar page or widget

Status: RESOLVED FIXED

(SeaMonkey :: Sidebar, defect, P3)

Description

[Blake Ross]

Build ID: 2000082408

When clicking on a tab in the Sidebar, focus should then switch to the sidebar 
page, or a (the first?) widget in the panel/page.  This is important in a 
couple of ways.  For example, if you select text in a sidebar panel and then 
select text in a website loaded in the adjacent viewport, the selection in the 
sidebar panel becomes unfocused (gray) and you then have no way to refocus it 
again.  Easily futurable, though (unless easy fix -- I imagine it wouldn't be 
all that hard, however)

Comment 1

[Blake Ross]

reassigning to matt and wondering why this wasn't done earlier in a mass 
change...

Comment 2

[Jesse Ruderman]

This bug blocks a nsbeta1 blocker, so nominating this bug for nsbeta1.

Comment 3

[matt]

german, can you comment on this bug.
In my notes you say it's invalid.
Am i correct?

Comment 4

[shrirang khanzode]

spam : changing qa to sujay (New Sidebar QA)

Comment 5

[german]

Quick digression: In the longer-term scheme of things I'd want a keyboard 
accessible general way (like the tab key) to cycle the main focus a la mail 
three pane, url bar -> page content -> sidebar tab (arrow keys to select) -> 
sidebar content if tab-able -> url bar...Spec following on that very soon.
So I guess we have to carefully think about how we want to implement this
without screwing in the longer run. So what do you think that for now we set the
focus only when there is a text entry area in that sidebar tab to fix the
problem described here for the short term? Any objections?

PS: Internally at Netscape I have a phonebook tab (XUL) that does display the
proper behavior, it focuses on the first text entry field. However a bugzilla
tab I am also using greys the text field out and the search panel does not grey
it out but also does not focus the query text field. Clearly this should be
cleaned up to have one consistent behavior, but I wonder why this seems tab
dependent?

Comment 6

[Jesse Ruderman]

> Quick digression: In the longer-term scheme of things I'd want a keyboard 
> accessible general way (like the tab key) to cycle the main focus a la mail 
> three pane, url bar -> page content -> sidebar tab (arrow keys to select) -> 
> sidebar content if tab-able -> url bar...Spec following on that very soon.

In bug 30864, mpt (and several others) suggested ctrl+tab for switching among 
frames in the content area, the location bar, and the sidebar.

I'd prefer if Ctrl+Tab took me straight to the content area of the active 
sidebar, and there was another key combo to move to another sidebar panel.

> So what do you think that for now we set the focus only when there is a text
> entry area in that sidebar tab to fix the problem described here for the 
> short term? 

Is that an easy way to do that?  Sidebars can be at least xul and html, and the 
code can't know which textbox in the panel should be focused.  What about long 
text that users might want to scroll through?

I'm also worried that sidebars might end up depending on that hack, making it 
difficult to remove.

Comment 7

[Paul Chen]

Mass moving most of mozilla0.9 bugs to mozilla0.8.1

Comment 8

[german]

Jesse your point is valid. The hack might not be worth it, better do it right.

Comment 9

[Paul Chen]

nav triage team:

Marking nsbeta1- but nsCatFood, it's important, but how important?

Comment 10

[Viswanath Ramachandran]

kevin: shd this be a nsCatFood+ bug?

Comment 11

[kmurray1115]

If it allows printing tabs, yes.

Comment 12

[Viswanath Ramachandran]

this bug is not related to the ability to print tabs we think. Lets minus it. 

Comment 13

[Paul Chen]

nav triage team:

Moving to mozilla0.9.2 and reassigning to Samir

Comment 14

[Keyser Sose]

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

Comment 15

[Viswanath Ramachandran]

sidebar+pdt triage: this is deferred till m1.0 at least. 

Comment 16

[Jesse Ruderman]

I can think of two possible ways to fix this bug:

1. When the user clicks a sidebar tab, record some state saying "when this 
finishes loading, I'm going to focus it".  Once the sidebar finishes loading, 
look at its DOM tree to determine whether it contains an HTML or XUL textbox.  
If it does, focus the first textbox; otherwise, focus the sidebar content area.

2. When the user clicks a sidebar tab, immediately focus the sidebar content 
area.  Allow the sidebar content to move focus to a textbox using javascript, 
just like a search engine's front page would do.  (Bug 76621, "sidebar elements 
should not grab focus from other parts of window", will allow sidebar panels to 
use textbox.focus() without stealing focus when the window is first created.)

I prefer 2, because
- Focus doesn't jump around as much, and ends up in the right place sooner.
- It allows me to use a page like http://www.cs.hmc.edu/~jruderma/js/calc/ as 
both normal content and a sidebar panel, using the same mechanism to focus the 
textbox as the page loads.  (The code to focus the textbox as the page loads is 
commented out because of bug 76621.)
- It's possible to introduce security holes by having chrome javascript look 
through the DOM tree of content.

Comment 17

[Peter Trudelle]

->moz101

Comment 18

[Vedran Miletic]

retargeting

Comment 19

[timeless]

.

Comment 20

[[e-mail]]

Attachment: Proposed patch

Re comment 16: The sidebar doesn't actually prohibit panels from stealing
focus, instead it defines an attribute for the element that the panel would
like to have focus, and it's up to the panel author to use that element instead
of stealing focus. However, switching panels should still default to the
content area, so I'm attaching this patch to address that issue.

Comment 21

[Pierre Chanial]

I'd rather put |this.contentWindow.focus();| in an |else| statement if
|elementToFocus| is not the empty string to avoid focusing twice.
Bonus if you add sth like dump("Element to focus not found!\n") if |element| is
not found (not |debug()| ).

Comment 22

[Pierre Chanial]

s/is not the empty string/is the empty string

Comment 23

[[e-mail]]

Attachment: Addressing review comments

Comment 24

[Alec Flett]

Comment on attachment 134631 [details] [diff] [review]
Addressing review comments

hmmm.. dump huh? I thought we tried not to dump anymore?
sr=alecf without the dump, or with a reasonable explanation.

Comment 25

[[e-mail]]

alecf: pch requested the dump in his review comment (don't know why).

Comment 26

[Pierre Chanial]

For internal use, I agree that dump should be avoided, but we can have sidebar
by third parties. Dumping an error message can save time if the sidebar creator
mistype the element.
Anyways, that's to your appreciation, my review stand in both cases.

Comment 27

[[e-mail]]

Fix checked in.