Closed Bug 990045 Opened 10 years ago Closed 10 years ago

Don't Auto-focus the Save Password prompt

Categories

(Firefox :: Keyboard Navigation, defect)

Product:
Firefox
Component:
Keyboard Navigation
Type:
defect
Priority:
Not set
Severity:
major
Points:
3

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 35
Iteration:
35.2

People

(Reporter: MarcoZ, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

focusNextFrame should special-case panels and notificationpopups, with the latter getting noautofocus by default, f?MarcoZ
4.76 KB, patch
enndeakin
: review+
MarcoZ
: feedback+
Details | Diff | Splinter Review 
STR:
1. Go to a page where you can enter a user name and password and you haven't saved one before.
2. Enter both and click Submit.
3. Save password prompt appears.

Expected: Save password prompt appears, but keyboard focus should stay in the newly loading document.
Actual: Save Password prompt appears, but keyboard focus jumps into the panel, and you can tab to the buttons straight away.

This defeats the purpose of the non-modal doorhangers. Keyboard focus should only go into the panel if the Alt+R shortcut is pressed, or it is being navigated to via F6 cycle.
There are two issues here:

1. Ensure that noautofocus is set on the popup.
2. When nothing (the document) is focused, tab navigation goes into a popup if one happens to be open. I think you are implying that this shouldn't happen and should navigate through the page.
(In reply to Neil Deakin from comment #1)
> There are two issues here:
> 
> 1. Ensure that noautofocus is set on the popup.
> 2. When nothing (the document) is focused, tab navigation goes into a popup
> if one happens to be open. I think you are implying that this shouldn't
> happen and should navigate through the page.

Correct, but I was under the impression that 2 is the result of 1. So if it is not auto-focused, tab would automatically navigate through the page. You're saying this is not the case?
From my testing, I don't think it's currently possible to get to this prompt with f6. That needs to be fixed as well.
(In reply to Marco Zehe (:MarcoZ) from comment #2)
> Correct, but I was under the impression that 2 is the result of 1. So if it
> is not auto-focused, tab would automatically navigate through the page.
> You're saying this is not the case?

Assume that noautofocus is set on the popup. If the page is focused but nothing within the page is focused, pressing tab will navigate into the popup. But if something within the page such as a text field is focused, then pressing tab will navigate as usual from the text field and one must use F6 to navigate into the popup.

As the save password prompt appears after submitting a form, a new page will typically be loaded which will cause a focus shift to the page, and most pages don't focus anything afterwards, so pressing tab will navigate into the popup.
We should at least just set noautofocus="true" on the doorhangers, and I'm marking backlog+ for that.

(In reply to Neil Deakin from comment #4)
> As the save password prompt appears after submitting a form, a new page will
> typically be loaded which will cause a focus shift to the page, and most
> pages don't focus anything afterwards, so pressing tab will navigate into
> the popup.

Is this an issue we can fix on the frontend, while preserving navigation with F6?
Flags: needinfo?(enndeakin)
Flags: firefox-backlog+
I'm not sure what you're asking. Do you have a suggestion for alternate behaviour?
Flags: needinfo?(enndeakin)
(In reply to Neil Deakin from comment #6)
> I'm not sure what you're asking. Do you have a suggestion for alternate
> behaviour?

If the doorhanger is technically a separate window, shouldn't it just not be part of the tab order, while you switch with F6? Marco?
Flags: needinfo?(marco.zehe)
Right now, it *is* a part of the tab order, right between the Google Search box and the main document. The thing is that before saving the password, the user one may want to review whether the login was successful, and then decide that the password should be saved. Sighted people can usually see that right away, screen reader users will have to read through some of the page to find that information. Focus may then not be near the beginning of the document at all any more, but transitioned way into the main content area. The way to go would, in the ideal world, be to just press shift+f6 and land on the doorhanger. One could then tab through the options and act on the password prompt.
Flags: needinfo?(marco.zehe)
> The way to go would, in the ideal world, be to just press shift+f6 and
> land on the doorhanger. One could then tab through the options and act on
> the password prompt.

Now I'm confused. You can already do switch to the password prompt with F6 and Shift+F6.
I'm confused. For me, the remember password prompt isn't in the f6 tab order, nor is it after the location bar. If the document doesn't steal focus, I can get to the doorhanger controls with one press of the tab key, which shouldn't happen.
As per Gijs's request, NI'ing him on doorhanger-keyboard-accessibility-related bugs.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to James Teh [:Jamie] from comment #10)
> I'm confused. For me, the remember password prompt isn't in the f6 tab
> order, nor is it after the location bar. If the document doesn't steal
> focus, I can get to the doorhanger controls with one press of the tab key,
> which shouldn't happen.

While the doorhanger is open, it is in the F6 order for me on OS X, but not on Windows.

Fixing the noautofocus shouldn't be hard (ie add an attribute to the relevant panel), but if the panel is then hard to reach, that might not be the right thing.

Here's some concrete STR on Windows:


1. Open http://jsbin/sarido/1/edit in a new tab

2. Enter bogus credentials in the "login form" on the right (making this up to just have something simple to test with) and hit enter / click the "Login" button.

3. The "Would you like to remember the password for "foo" on jsbin.com" doorhanger pops up

4. Hit F6. The location bar is focused

5. Hit F6 again. The location bar keeps focus.

6. Hit Shift-F6. The page gets focus. You can tab around and see link text in the bottom corner of the browser.

7. Hit Shift-F6 again. The location bar is focused again.


There doesn't seem to be a way to use F6 to switch to the panel (which remains open throughout). Neil, can you reproduce this and/or do you know why this would be?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(enndeakin)
It actually does focus the right thing, but the code in browser.js::focusNextFrame tries to focus and select the location field after navigating, causing the focus to move back there.

Ideally, moveFocus should somehow handle this internally somehow rather that requiring a special handler. Alternatively, focusNextFrame should check if the focus is inside a popup before trying to focus the location field.

I'm not sure why the key elements that invoke focusNextFrame are non-Mac specific though. Maybe dao would remember why.
Flags: needinfo?(enndeakin) → needinfo?(dao)
(In reply to Neil Deakin from comment #13)
> I'm not sure why the key elements that invoke focusNextFrame are non-Mac
> specific though. Maybe dao would remember why.

I don't think this was intentional.
Flags: needinfo?(dao)
(In reply to Neil Deakin from comment #13)
> It actually does focus the right thing, but the code in
> browser.js::focusNextFrame tries to focus and select the location field
> after navigating, causing the focus to move back there.
> 
> Ideally, moveFocus should somehow handle this internally somehow rather that
> requiring a special handler.

How would moveFocus know what to focus when, ie can you provide more detail as to how you think this should work? Does it make sense to use the simpler-sounding workaround of updating focusNextFrame to not move focus out of popups for now?
Flags: needinfo?(enndeakin)
> How would moveFocus know what to focus when, ie can you provide more detail
> as to how you think this should work? Does it make sense to use the
> simpler-sounding workaround of updating focusNextFrame to not move focus out
> of popups for now?

Changing focusNextFrame seems fine.

I don't really have an answer for how moveFocus would do this.
Flags: needinfo?(enndeakin)
Still need to check this on Windows, but I presume this is more or less what we want here. Marco, can you check if this works better for you?
Attachment #8489364 - Flags: feedback?(mzehe)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 35.1
Flags: qe-verify?
Comment on attachment 8489364 [details] [diff] [review]
focusNextFrame should special-case panels and notificationpopups, with the latter getting noautofocus by default,  f?MarcoZ

Review of attachment 8489364 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me on Windows.
Attachment #8489364 - Flags: review?(enndeakin)
Flags: qe-verify? → qe-verify+
Comment on attachment 8489364 [details] [diff] [review]
focusNextFrame should special-case panels and notificationpopups, with the latter getting noautofocus by default,  f?MarcoZ

F6 now reliably toggles through the open doorhanger, the document, and the awesome bar.

One thing I did notice is that, when the password prompt appears, and the new page is loaded, focus doesn't seem to go to the document. Instead, when now pressing tab, it again lands on the password doorhanger, as if that's the first thing in the document's tab order. So when removing the auto focus, it should be made sure that focus is on the new document when it loads and not somewhere in limbo.
Attachment #8489364 - Flags: feedback?(mzehe) → feedback+
Hi Gijs, can you provide a point value.
Flags: needinfo?(gijskruitbosch+bugs)
Oops, forgot that. Thanks!
Points: --- → 3
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8489364 - Flags: review?(enndeakin) → review+
(In reply to Marco Zehe (:MarcoZ) from comment #19)
> One thing I did notice is that, when the password prompt appears, and the
> new page is loaded, focus doesn't seem to go to the document. Instead, when
> now pressing tab, it again lands on the password doorhanger, as if that's
> the first thing in the document's tab order.

Isn't this a more general problem with where keyboard focus goes when loading a page? ISTR we have general issues with whether or not we manage to focus the page and/or the location bar when loading a new page/tab. That seems like a separate problem from this bug.

Can you confirm that landing this as-is an improvement, or do you think the current state is better than landing the patch without fixing the "limbo focus" immediately?
Flags: needinfo?(mzehe)
I think landing the current patch is OK, but this problem should not be forgotten. ;)
Flags: needinfo?(mzehe)
Iteration: 35.1 → 35.2
remote:   https://tbpl.mozilla.org/?tree=Try&rev=1f88b3f4e20d
remote: Alternatively, view them on Treeherder (experimental):
remote:   https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1f88b3f4e20d
https://hg.mozilla.org/mozilla-central/rev/c417a2154756
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
QA Contact: cornel.ionce
Verified fixed on Windows 7 64-bit, Windows 8.1 64-bit, Ubuntu 12.04 and Mac OS X 10.8 using latest Nightly, build ID: 20140924030204.

> One thing I did notice is that, when the password prompt appears, and the
> new page is loaded, focus doesn't seem to go to the document. Instead, when
> now pressing tab, it again lands on the password doorhanger, as if that's
> the first thing in the document's tab order. So when removing the auto
> focus, it should be made sure that focus is on the new document when it
> loads and not somewhere in limbo.

Is there a bug logged for this issue mentioned by Marco in comment 19?
Status: RESOLVED → VERIFIED
Blocks: 1073551
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: