302193 - Print Preview Dialog: tabbing gets stuck when launched with some pages

Status: RESOLVED FIXED

(Firefox :: Keyboard Navigation, defect)

Description

[Dan Kinnunen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

Print Preview Dialog has no visual focus when the dialog is opened. Nothing
happens when the user presses the tab to navigate.

Reproducible: Always

Steps to Reproduce:
1. Alt + F for file menu
2. Press V for 'print preview'
3.

Actual Results:  
Tabbing results in no navigation of the 'print preview'.

Expected Results:  
Should have a visual focus of the 'print preview'.

Comment 1

[Aaron Leventhal]

We don't usually show focus in a content pane unless the user tabs there. When a
screen opens with a content pane, it is assumed that focus is in the content
unless it is shown somewhere else. That is the same as IE and other browsers.
Otherwise there would always be a visually annoying focus outline in the content
pane.

Comment 2

[Aaron Leventhal]

I have noticed an intermittent problem that I think is just related to google
and anything that sets focus in an onload.

Sometimes when I load Google into print preview and hit tab, focus actually goes
to the "Google Search" button but gets stuck there. You can't tab away.

Comment 3

[Dan Kinnunen]

We agree with the comment about initial focus to the content pane.  The question
still remains, how do you move keyboard focus from the content pane to the
toolbar?  There is no menu bar in this window, and no apparent way to navigate
to the toolbar using the keyboard.  The inability to navigate the toolbar is the
real problem here.  (By the way, in Internet Explorer, you can use the tab key
to move from the content pane to the tool bar in the print preview window.)

Regarding the Google tabbing issue, I see something similar in the Yahoo mail
sign-in page.  Sounds like a similar issue, if only because it involves the odd
implementation of the Tab key in the print preview pane.  I can think of no good
reason to allow the tab key to navigate elements in the content pane in a print
preview pane.

Comment 4

[Aaron Leventhal]

Try another page. Or close print preview and open a 2nd time. The tab key will
then move into the toolbar.

Also, I think that's why tab gets stuck. There's a stylesheet which is supposed
to turn tab navigation off in the content for print preview, but the onload
setfocus command to the textfield must be confusing that code.

Comment 5

[Dan Kinnunen]

We see the behavior you mention in google.com and mail.yahoo.com.  In both cases
the tab key gets "stuck" in the preview content pane, instead of moving focus up
to the toolbar.

Closing the print preview and reopening it did move the focus up to the toolbar.
 That is interesting, but of course, it is not an acceptable workaround for the
problem.

Comment 6

[Aaron Leventhal]

Morphing this bug since we already have bug 281932 for key nav of the toolbar.

Comment 7

[Aaron Leventhal]

I also found that I could click to focus textfields in print preview.

Comment 8

[Aaron Leventhal]

Attachment: Simple patch: Return early from esm:SetFocusedContent() when in print preview

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 190786 [details] [diff] [review]
Simple patch: Return early from esm:SetFocusedContent()  when in print preview

Someone who actually understands the focus code (mats, bryner, maybe neil)
needs to review this...  I don't know enough about the code to judge whether
this is the right patch (eg, why not return NS_OK?)

Comment 10

[Aaron Leventhal]

Comment on attachment 190786 [details] [diff] [review]
Simple patch: Return early from esm:SetFocusedContent()  when in print preview

If one of you gets to this and find it totally reasonable could you r+sr= it
since we're short on time?

Comment 11

[Aaron Leventhal]

Attachment: Same thing, but allow SetFocusedContent(nsnull)

Comment 12

[[e-mail]]

Well, this does seem to be an improvement over the current code.
I don't suppose you know who the "bogus" caller(s) is/are?
Also, any ideas on the forward tabbing not cycling issue?

Comment 13

[Aaron Leventhal]

(In reply to comment #12)
> Well, this does seem to be an improvement over the current code.
> I don't suppose you know who the "bogus" caller(s) is/are?
It's the generic focus that happens because a new window needs to get focus when
the old non-preview content window is destroyed.

> Also, any ideas on the forward tabbing not cycling issue?
What's the forward tabbing not cycling issue?

Comment 14

[Aaron Leventhal]

Neil, there is one place in the tab cycle where we don't show focus when focus
is on the print preview content pane, but one more tab after that starts you at
the beginning of the toolbar again.

Comment 15

[[e-mail]]

Comment on attachment 191208 [details] [diff] [review]
Same thing, but allow SetFocusedContent(nsnull)

OK, it must have been my build; another tree works fine.

Comment 16

[Mats Palmgren (inactive)]

Attachment: Testcase

Comment 17

[Mats Palmgren (inactive)]

(In reply to comment #13)
> (In reply to comment #12)
> > Well, this does seem to be an improvement over the current code.
> > I don't suppose you know who the "bogus" caller(s) is/are?
> It's the generic focus that happens because a new window needs to get focus when
> the old non-preview content window is destroyed.

I don't think so, tabbing is working fine for other pages. The problem occurs
because google.com has an "onload=input.focus()".

Comment 18

[Mats Palmgren (inactive)]

(In reply to comment #14)
> Neil, there is one place in the tab cycle where we don't show focus when focus
> is on the print preview content pane, but one more tab after that starts you at
> the beginning of the toolbar again.

The reporter expected visual feedback that the focus is on the
document - I think that's reasonable. (spawn off a new bug?)

Comment 19

[Mats Palmgren (inactive)]

Comment on attachment 191208 [details] [diff] [review]
Same thing, but allow SetFocusedContent(nsnull)

>+  if (aContent &&
>+      (!mPresContext || mPresContext->Type() == nsPresContext::eContext_PrintPreview)) {
>+    return NS_ERROR_FAILURE;
>+  }

I think we should return NS_OK since we don't "fail", we "ignore".
With that, r=[e-mail]

I also noted an additional problem for pages that call focus():
most keyboard commands (PAGE_DOWN etc.) does not work initially,
but if you do TAB followed by SHIFT-TAB they start to work.
(this occurs in a current build on Linux and the patch
does not seem to fix that, it also occurs in Mozilla 1.7.3 so it's
not a regression). (spawn off a new bug?)

Finally, we currently trigger onfocus/blur DOM callbacks in
(non-interactive) Print/PrintPreview contexts (also with the patch).
I think we should consider not doing that (essentially ignoring
DOM focus()/blur() completely).

Comment 20

[Aaron Leventhal]

> The reporter expected visual feedback that the focus is on the
> document - I think that's reasonable. (spawn off a new bug?)
Yes, I suppose it should be a general fix for content areas. They need to show
focus when you first tab to them, and then the focus outline goes away as you
start to do something else like scroll.

Comment 21

[Aaron Leventhal]

Checking in nsEventStateManager.cpp;
/cvsroot/mozilla/content/events/src/nsEventStateManager.cpp,v  <-- 
nsEventStateManager.cpp
new revision: 1.594; previous revision: 1.593
done

Comment 22

[Dan Kinnunen]

Actual Results: When you open print preview the FIRST time after starting
Firefox, you cannot tab to the tool bar buttons.
Expected Results:  You should be able to TAB to the toolbar.  (If you close the
print preview and reopen, it works okay.)

Comment 23

[Aaron Leventhal]

This is FIXED for me.

Comment 24

[Dan Kinnunen]

Update, this reported problem has been fixed in 08/19 nightly build. Changing
status to "FIXED".