120148 - Tabs don't remember which element [frame, textfield, etc] was focused when switching tabs

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Jonas Jørgensen]

* Go to a page that uses frames
* Switch to another tab
* Switch back
* Press the up or down arrow - the page doesn't scroll.

Comment 1

[David Hyatt]

Reassigning to new component owner.

Comment 2

[jag (Peter Annema)]

-> future, helpwanted

Comment 3

[Jonas Jørgensen]

If focus is in a textfield, and I switch to another tab and then switch back,
the textfield doesn't get focused again. That's probably the same bug as this -
tabbrowser needs to remember which element was focused the last time the tab was
active. That element could either be an HTML element (like a textfield) or a
page inside the frameset.

Adding access keyword and updating summary.

Comment 4

[Jonas Jørgensen]

If focus is in a textfield, and I switch to another tab and then switch back,
the textfield doesn't get focused again. That's probably the same bug as this -
tabbrowser needs to remember which element was focused the last time the tab was
active. That element could either be an HTML element (like a textfield) or a
page inside the frameset.

Adding access keyword and updating summary.

Comment 5

[Jonas Jørgensen]

Oops... sorry about that.

Comment 6

[jag (Peter Annema)]

We should be able to do this by storing the focused element/window and resetting
those when switching back to the tab.

Comment 7

[jjkarppi]

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

Comment 8

[sairuh (rarely reading bugmail)]

aaronl, should this go to you since you're working on similar issues?

Comment 9

[Aaron Leventhal]

No, this should go to bryner or jaggernaut. I'll pick up some of their
focus/navigation bugs if I get the chance, but now isn't a good time.

Comment 10

[Peter Trudelle]

nsbeta1+ per ADT triage team, ->1.0

Comment 11

[sairuh (rarely reading bugmail)]

ended up kinda testing this issue during the course of using a 2/20 test build
for bug 66597 and bug 120023.

a. caret browsing on, two tabs: switching btwn the tabs seems to preserve focus
in that the caret remains visible. if i click w/the mouse to switch tabs, the
focus ring around a focusable item persists. but if i switch using
ctrl+pageUp/pageDown, the focus ring disappears [but the caret does not].

b. caret browsing off, two tabs: if i click w/the mouse to switch tabs, the
focus ring around a focusable item also persists. but if ctrl+pageUp/pageDown is
used, the focus ring is lost. however, in both cases --as with case (a)-- focus
is where i last left it, since tabbing/shift+tabbing proceeds as exoected anyhow.

c. hrm, cannot seem to reliably repro the weird case [bug 66597 comment 45 part]
where the previously focused item still has the focus ring [when it's not
supposed to], or other erratic oddments like seeing focus rings on multiple
items... sigh. will post recipe if i can get either of these to occur reliably...

Comment 12

[Jaime Rodriguez, Jr.]

nsbeta1- per ADT triage. We believe that users are able to recover by tabbing to
the ext field. Pls renominate, if this is not true.

Comment 13

[Peter Trudelle]

It seems to me like users could be as lost as the focus.  For instance, on the
current page, a it took 126 tabs to get back to the text area I was in before
switching tabs, though if I still used the product rather than switching to IE,
I might eventually learn to do tab + shift tab on such occasions.  Aaron?

Comment 14

[Aaron Leventhal]

Yes, there's also a bug somewhere that the focus is not remembered when you go
back or forward in history.

However, I wouldn't call this nsbeta1+. It's a usability irrititation, which
should eventually be fixed, perhaps not now. If this is a huge concern to a
user, they don't have to use tabbed browsing.

Comment 15

[Boris Zbarsky [:bzbarsky]]

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

Comment 16

[Ajay Gautam]

Found more info on this bug: Steps I followed:
1. Open mozilla
2. go to a website
3. CTRL-T -> go to another website
4. Click on the 1st tab (did not click inside the client area of 1st tab)
5. (Forgot about this bug and) started typing.
6. Pressed Enter: Thats when I noticed that the Enter went to the 2nd tab!!

I guess the Enter should have gone to the 1st tab. Looks like the tab in whihc
one clicks the mouse retains focus unless you click "inside" the contents of the
tab in which you want the focus to go.

I hope I have not made this very confusing. Please let me know if I can provide
more details.

Browser details:
Build ID from title Bar: 2002041203
Help->About
Mozilla 0.9.9+
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.9+) Gecko/20020412

Thanks for creating such a fine product.

Ajay Gautam :)

Comment 17

[Boris Zbarsky [:bzbarsky]]

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

Comment 18

[Boris Zbarsky [:bzbarsky]]

So... Is there an HTML equivalent of the command dispatcher?  It seems like the
simple solution to this situation is to simply store the element that is focused
when switching away from a tab and to make sure that it gets focused when we
switch back (have to watch out for unwanted onfocus events here, though).

The problem is getting the "currently focused element" out of an HTML document....

Comment 19

[Aaron Leventhal]

Do you want to get the currently focused element in Javascript or C++?

I can't find a js way to do this:
    nsCOMPtr<nsIScriptGlobalObject> ourGlobal;
    document->GetScriptGlobalObject(getter_AddRefs(ourGlobal));
    nsCOMPtr<nsPIDOMWindow> ourWindow(do_QueryInterface(ourGlobal));
    if (ourWindow) 
      ourWindow->GetRootFocusController(getter_AddRefs(focusController));

Comment 20

[Boris Zbarsky [:bzbarsky]]

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

Comment 21

[Boris Zbarsky [:bzbarsky]]

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

Comment 22

[sairuh (rarely reading bugmail)]

renominating for buffy.

Comment 23

[Samir Gehani]

Nav triage team: nsbeta1+/adt3

Comment 24

[Brian Ryner (not reading)]

Yeah, we never implemented focus memory for tabs the same way as it works for
windows.  I'd actually be quite hesitant to do that (create a FocusController
per tab) because it seems very risky.

I think what bz suggests is the way to go.  We need to make sure to restore the
focused DOMWindow as well as the focused element.  Would installing a capturing
focus listener on the <browser> allow you to track focus changes?

Comment 25

[Jesse Ruderman]

See also bug 124750, "other tab steals focus with javascript textbox.focus()".

Comment 26

[Jesse Ruderman]

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

Comment 27

[Andreas Kunz]

Is this a duplicate of bug 112081 ? Or vice-versa as there are more comments in
this bug here and the summary is not as specific...

Comment 28

[Boris Zbarsky [:bzbarsky]]

Actually, that bug is a purely visual issue.  This bug is about the focus not
being persisted.  Fixing this may fix the visual problem, of course.

Comment 29

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 30

[Zook Valem]

Target Milestone, should it be changed to future??  We're already way past the
Mozilla 1.1 alpha milestone...

Comment 31

[Jesse Ruderman]

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

Comment 32

[jag (Peter Annema)]

Attachment: Remember where focus is when switching tabs

Comment 33

[jag (Peter Annema)]

Attachment: Remember where focus is when switching tabs (without exceptions)

So if the element isn't focusable, it won't have a focus method, in which case
I can fall back to setting focus on the focusedWindow, instead of always
setting focus on it. Also use lookupMethod to get at the original focus method
instead of a user defined one.

Comment 34

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

Please add a comment about where setfocus is false, and why you want to treat
that differently.  It confused me for a minute until I realized what was going
on.  Looks good otherwise.  r=caillon.

>+            if (this.getAttribute("setfocus") != "false") {
>+              next.focus();
>+              document.commandDispatcher.advanceFocusIntoSubtree(next);
>+            }

Comment 35

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

Please add a comment about where setfocus is set, and why you want to treat
that differently.  It confused me for a minute until I realized what was going
on.  Looks good otherwise.  r=caillon.

>+            if (this.getAttribute("setfocus") != "false") {
>+              next.focus();
>+              document.commandDispatcher.advanceFocusIntoSubtree(next);
>+            }

Comment 36

[jag (Peter Annema)]

            // set to false by users of <tabs> (e.g. tabbrowser) to indicate
            // they don't want us to set focus to the selected panel 

Comment added.

Comment 37

[Boris Zbarsky [:bzbarsky]]

Are we calling window.focus there?  Will that not raise the Mozilla window when
called?  Could you please test that switching tabs while the Mozilla window is
not on top does _not_ raise it?

Comment 38

[Roman R.]

Boris, I don't think you should be concerned with that as you cannot switch 
tabs if the Mozilla window is not on top.

Comment 39

[jag (Peter Annema)]

bz: like Roman R. said, it's kinda hard to switch tabs while the window is not
on top, since there's no programmatic way of doing it. Maybe if you try really
hard you can switch to another tab, then switch the window to the background
before the focus() call gets processed, but it's a bit of an edge case and I'm
not changing anything about that aspect of the code (though I'd be happy to
address that in another bug if deemed necessary).

Comment 40

[Christopher Aillon (sabbatical, not receiving bugmail)]

Actually, on linux, it is possible to do that if you have keyboard focus on
whatever the mouse is over and mozilla is a background window.  Ctrl+PgUp/PgDown.

Comment 41

[Boris Zbarsky [:bzbarsky]]

Um.. That wasn't a theotical question, guys.  If you're not using a
windowmanager that enforces raise-on-focus (most Linux windowmanagers) then it's
trivial to switch tabs without the Mozilla window being on top.  And Mozilla
raising itself when you did that would be totally unacceptable.

Comment 42

[jag (Peter Annema)]

I don't think we raise on linux when we focus a window. Easy enough to test
though with any recent nightly, we call focus with and without this patch, the
only difference is the element focused.

Comment 43

[Boris Zbarsky [:bzbarsky]]

We certainly raise on Linux if window.focus() is called.  Focusing things other
than the window does _not_ raise.  It may also not raise if called from chrome.
 The point is, it would be good if someone were to test that before checking
this in....

Comment 44

[Brian Ryner (not reading)]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

I don't think this will cause us to raise the window when you switch tabs in
any case where we don't currently.  Looks fine to me.  sr=bryner.

Comment 45

[jag (Peter Annema)]

I'll test that before checking in. If the user can somehow make the
focusedWindow be the XUL window with focusedElement being null, then that's one
case where we could end up (effectively) calling window.focus();

Comment 46

[Asa Dotzler [:asa]]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

a=asa (on behalf of driver) for checkin to 1.4b

Comment 47

[jag (Peter Annema)]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

I can get into a state where if you load a (big/slow) page in a tab, then
switch to another tab and put focus in the URL bar, then switch back to the tab
that's still loading, focus will stay in the URL bar instead of going to
content. I think resetting the cached focusedWindow and focusedElement from
onLocationChange will "fix" this.

Comment 48

[Samir Gehani]

adt: nsbeta1-

Comment 49

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.4) Gecko/20030624] (v1.4rc3)

Bug still there.


Any news since v1.4b checkin approval in comment 46 ? (2 months ago)

Comment 50

[Boris Zbarsky [:bzbarsky]]

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

Comment 51

[Matt Greco]

This bug still appears to be there using v1.4...

Comment 52

[Jo Hermans]

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

Comment 53

[Boris Zbarsky [:bzbarsky]]

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

Comment 54

[Vaclav Dvorak]

How about driving this in for 1.6a, please? Thanks!

Comment 55

[Hanspeter Niederstrasser]

Attachment 121644 [details] [diff] no longer patches cleanly against the trunk.  I've fixed the
patch to match the current source and it does _not_ fix the bug on Firebird (on
Mac OS X built today): tabs still forget their focus.  I'm attaching the
corrected patch if someone can try it on Seamonkey.

Comment 56

[Hanspeter Niederstrasser]

Attachment: Remember where focus is when switching tabs (updated)

Updated patch to patch cleanly.

Comment 57

[Hanspeter Niederstrasser]

Attachment: Remember where focus is when switching tabs (updated)

Updated patch to patch cleanly.

Comment 58

[Hanspeter Niederstrasser]

Comment on attachment 132484 [details] [diff] [review]
Remember where focus is when switching tabs (updated)

Sorry about the double patch.

Comment 59

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 121644 [details] [diff] [review]
Remember where focus is when switching tabs (without exceptions)

Hm, when you say you tried this patch, did you mean you made equivalent changes
to equivalent files in firebird?  Of course this patch will not work in
Firebird without any munging because it is a Seamonkey only patch....

Comment 60

[Vaclav Dvorak]

Comment on attachment 132485 [details] [diff] [review]
Remember where focus is when switching tabs (updated)

Obsoleting redundant patch (identical to previous, AFAICS).

Comment 61

[Vaclav Dvorak]

Comment on attachment 132485 [details] [diff] [review]
Remember where focus is when switching tabs (updated)

Oops... I'm not alone here. Sorry.

Comment 62

[Hanspeter Niederstrasser]

Comment on attachment 132485 [details] [diff] [review]
Remember where focus is when switching tabs (updated)

Christopher Aillon:
> Hm, when you say you tried this patch, did you mean you made equivalent 
> changes to equivalent files in firebird?  Of course this patch will not 
> work in Firebird without any munging because it is a Seamonkey only patch....

Sorry.	I was partly confused.	Here's a (hopefully) clearer description of
what I've done:

1) Attachment 121644 [details] [diff] no longer patches cleanly to the trunk because of other
changes to the patched files since the patch was written.
2) Because the patch had bit rotted, I fixed the patch to patch cleanly without
errors (this is attachment 132485 [details] [diff] [review]).
3) I tried the fix against Firebird, but it didn't work.  As you've now pointed
out, the patch only patches seamonkey code.  I'm still working on figuring out
what will affect which project.  Lesson learned.

4) I have now built Seamonkey with my patch and it does fix the loss of focus
when tabs are switched.

The patch I've submitted does the same exact modifications as the original
patch (attachment 121644 [details] [diff] [review]).  It just takes into account changes to the affected
files from the last 5 months.

Because the patch is the same otherwise, I'm requesting review so that we can
move to finally check this in.

Comment 63

[Christopher Aillon (sabbatical, not receiving bugmail)]

Can you answer the question that was raised in comment 37?  See ensuing
discussion up to comment 45.

Comment 64

[Hanspeter Niederstrasser]

From comment 63 (caillon):
> Can you answer the question that was raised in comment 37?

From comment 37 (bz):
> Could you please test that switching tabs while the Mozilla window is
> not on top does _not_ raise it?

The bottom Mozilla window is _not_ raised when its tabs are switched (Linux with
WindowMaker) by either clicking the tab or ctrl-pgup/pgdn.  Textboxes retain
their carets and respond to typing when that tab is made active.

Comment 65

[Boris Zbarsky [:bzbarsky]]

Great!  If the same is true when the thing that has focus is the main page and
not a textbox in it, then my worries are completely dispelled.

Comment 66

[Hanspeter Niederstrasser]

bz: If the page has focus (ie keyboard scrolling works) and I switch to another
tab where the focus is on a text area, switching back to the first tab keeps
focus on the document.

Comment 67

[Boris Zbarsky [:bzbarsky]]

Right.  What I meant was that if that does not cause the window to raise, then
things are good.

Comment 68

[Hanspeter Niederstrasser]

Ah.  The active window does not rise when switching to another tab, regardless
of where focus is on that tab (document, text box, URL bar).

Comment 69

[timeless]

checked in

Comment 70

[Hanspeter Niederstrasser]

This is fixed now after the checkin.  For the Firebird resolution of this, see
bug 224416.

Comment 71

[paul stanton]

ooi what firebird and mozilla releases will be bundled with this fix?

Comment 72

[Christian :Biesinger (don't email me, ping me on IRC)]

mozilla 1.6beta (or current nightyl nbuilds). firebird will have to wait until
bug 224416 is fixed