We should focus (something in) the document (ideally) or the URL bar. Now it's not clear where focus goes. (In reply to Marco Zehe (:MarcoZ) from bug 990045 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. 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.

Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws

Comment on attachment 8669071 [details] MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws https://reviewboard.mozilla.org/r/21119/#review19011 ::: toolkit/modules/PopupNotifications.jsm (Diff revision 1) > - popupnotification.setAttribute("noautofocus", "true"); How is adding the noautofocus attribute to the markup different than when it was added through the JS? Does it just come down to timing and the focus during binding creation or something like that? ::: toolkit/modules/PopupNotifications.jsm:872 (Diff revision 1) > + this.panel.removeAttribute("noautofocus", "true"); From this patch, it looks like the old code never removed the noautofocus attribute, so how was the focus getting placed in the notification before?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2) > Comment on attachment 8669071 [details] > MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless > explicitly opened using mouse or keyboard, r?jaws > > https://reviewboard.mozilla.org/r/21119/#review19011 > > ::: toolkit/modules/PopupNotifications.jsm > (Diff revision 1) > > - popupnotification.setAttribute("noautofocus", "true"); > > How is adding the noautofocus attribute to the markup different than when it > was added through the JS? Does it just come down to timing and the focus > during binding creation or something like that? > > ::: toolkit/modules/PopupNotifications.jsm:872 > (Diff revision 1) > > + this.panel.removeAttribute("noautofocus", "true"); > > From this patch, it looks like the old code never removed the noautofocus > attribute, so how was the focus getting placed in the notification before? The old code set the attribute on the popupnotification element, which is inside the panel. That has 0 effect. The new code sets and removes it on the element depending on whether focus needs to go into it (effectively only allowing that to happen if the user explicitly makes the doorhanger appear). Because of this being dynamic, I didn't add it to the markup, although come to think of it, it probably needs to be on the element from the start so it works the first time (as the patch as-is adds it when the popup is hidden).

Comment on attachment 8669071 [details] MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws (In reply to :Gijs Kruitbosch from comment #3) > Because of this being dynamic, I > didn't add it to the markup, although come to think of it, it probably needs > to be on the element from the start so it works the first time (as the patch > as-is adds it when the popup is hidden). Oh, I did add it to the markup already. Nevermind, then I just re-request review...

Comment on attachment 8669071 [details] MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws https://reviewboard.mozilla.org/r/21119/#review19021 Ok, I see that it is now being set on the panel instead of the popupnotification. Thanks for the clarification!

Jared, can you give me a second opinion on how safe you think this change is for uplift? I think it's safe enough to request aurora+beta uplift, but I also feel like I might be biased. Thoughts?

Reproduced this issue with Firefox 43 beta 4, build ID: 20151116155110 on Windows 10 x64. I can confirm this issue being fixed on latest Aurora 44 (build ID: 20151117004023). Tested on: *Windows 10 x64 *Ubuntu 14.04 x64 *Mac OS X 10.8.5

Oops, seems like the beta consideration here got dropped. That's a shame, but it's not too late. Considering this has baked for literally 1.5 month on nightly + aurora and I am unaware of regressions, going to request uplift for beta.

Comment on attachment 8669071 [details] MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws Approval Request Comment [Feature/regressing bug #]: setting focus appropriately when password doorhanger pops up [User impact if declined]: we do not set focus appropriately. This is problematic for keyboard and accessibility tool users [Describe test coverage new/current, TreeHerder]: nope! [Risks and why]: low, considering baking time (1.5 month now), lack of reported regressions, the size of the fix (ie very small), and that this is essentially removing no-op, broken code that we already landed to try to do exactly what this bug was about, and replaced it with code that, err, actually does it. :-) [String/UUID change made/needed]: nope!

Sigh, sorry for missing this needinfo.

I agree with the beta uplift request.

Comment on attachment 8669071 [details] MozReview Request: Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r?jaws Accessibility fix for recent regression, looks good to me. Please uplift to beta.

Tracking since this sounds like a regression, marking affected for 43.

This cause problems on beta: grafting 306688:310287f0e8f6 "Bug 1073551 - fix doorhangers to not steal focus unless explicitly opened using mouse or keyboard, r=jaws" merging toolkit/modules/PopupNotifications.jsm warning: conflicts during merge. merging toolkit/modules/PopupNotifications.jsm incomplete! (edit conflicts, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue) can you take a look, thanks!

Landed on beta: https://hg.mozilla.org/releases/mozilla-beta/rev/7434475a5d02

I was able to reproduce this issue with Firefox 43 beta 4, build ID: 20151116155110 on Windows 8 x86. This issue is no longer reproducible on Firefox 43 beta 8, build ID: 20151201152349 across platforms[1]. [1]Windows 8 x86, Mac OS X 10.11 and Ubuntu 14.04 x86.