User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0 Build ID: 20170125172332 Steps to reproduce: We have a page with third party javascript advertisement code, advertisement provider had an issue with their SSL certificate about month ago in short period (couple of hours) and code use iframes. Our pages starts "randomly" jumping for some users and focuses inside body after loading page, our review find that only Mozilla Firefox has this problem (50.1.0, but problem exists also in actual version). Problem is little complex because occurs only if page loading is blocked with some synchronous javascript code after problematic iframe and has sufficient height. 1) Page contains iframe with SSL certificate problem 2) document height is higher than browser vieport height (activated vertical scrollbar) 3) After that iframe page contains code which loads synchronously some blocking javascript which postpone onload of this page. 4) Page onload trigger scroll and focus to the iframe, which can be everywhere in page also can be invisible or have zero width/heigh I create page http://www.bavtese.info/firefox-issue.html with all conditions to replicate it Actual results: Page focus to the iframe with error message generated by Firefox from aboutNetError.xhtml template, it contains buttons with autofocus="true" HTML parameter http://www.w3schools.com/TAgs/att_button_autofocus.asp I found some source code in this mozilla-central repository on lines 621, 622 and 624 in file browser/base/content/aboutNetError.xhtml@2aede0a97bc6 https://hg.mozilla.org/mozilla-central/file/tip/browser/base/content/aboutNetError.xhtml#l621 From the line 632 this issue was fixed with javascript code but not for buttons with id="returnButton", id="openPortalLoginPageButton" and id="advancedButton" Expected results: Don't use autofocus="true" if error page is inside iframe.

[Tracking Requested - why for this release]: Regression due to autofocus in subframes Similar to Bug 748803 again, but different scenario :( Regression window: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=1300e56c184d106c8b0ce89a453c755bd73f6e7b&tochange=8e4fa465e2c6ab3cdf67db304a35135c7be55c08 Regressed by: Bug 1207107

Panos, is there someone on your team who can take a look?

Thanks, patch incoming.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d74aee24bddf6e6dea470b09972df589950dd45

Comment on attachment 8834911 [details] Don't autofocus buttons when about:certerror is embedded in an iframe (bug 1336352). https://reviewboard.mozilla.org/r/110690/#review111998 ::: browser/base/content/aboutNetError.xhtml:646 (Diff revision 1) > + for (buttonId of buttons) { > + var button = document.getElementById(buttonId); > - var parent = button.parentNode; > + var parent = button.parentNode; > - button.remove(); > + button.remove(); > - button.setAttribute("autofocus", "true"); > + button.setAttribute("autofocus", "true"); > - parent.appendChild(button); > + parent.appendChild(button); This will not work with the button-spacer div, certErrorAndCaptivePortalButtonContainer will end up with the button-spacer as the first child and the three buttons will follow it. We'll have to remember a sibling node and use insertBefore/After I think. In any case, autofocus should only be set on one button in the document (current code is bad :( ). I.e. we only need to set autofocus on errorTryAgain, and probably one of \{returnButton, openPortalLoginPageButton\} depending on what kind of page we're showing. It might be tricky to set autofocus here though, since we haven't even figured out what kind of page we're displaying at this point (initPage() hasn't been called yet). I think it depends on when exactly autofocus comes into play. If we can get away with setting the attribute later (once we know if it's a captive portal page), that would be convenient. Otherwise, first idea that pops into my head is to have a single <button> element in certErrorAndCaptivePortalButonContainer and dynamically set the ID and text label later. ::: browser/base/content/test/captivePortal/browser_captivePortal_certErrorUI.js:51 (Diff revision 1) > "Captive portal error page UI is visible."); > > info("Clicking the Open Login Page button."); > - doc.getElementById("openPortalLoginPageButton").click(); > + let loginButton = doc.getElementById("openPortalLoginPageButton"); > + is(loginButton.getAttribute("autofocus"), "true", "openPortalLoginPageButton has autofocus"); > + loginButton.click(); Hmm, a better test would be to have a page that has a button with autofocus="true", and an iframe with a cert error. Then check if the button outside the iframe has focus (it should).

Good catch about the node ordering, I fixed it in this version. I also discovered another button arrangement, when we discover that a pref reset might help fix things and made the right button autofocus in this case, too. I've manually tested all 4 cases (netError, certError, certError+prefReset, captivePortal) and added more checks in existing tests. As we discussed yesterday, I tried replicating the attached test case but got nowhere, since the timing introduced by the php script makes all the difference. If you have any ideas how to do that, I'm all ears. New try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0c34623018de06c5531cc89588fce3b538f5461

Tracking for 52/53/54+ - regression that we should address.

Comment on attachment 8834911 [details] Don't autofocus buttons when about:certerror is embedded in an iframe (bug 1336352). https://reviewboard.mozilla.org/r/110690/#review114106 Thanks! I don't have any ideas for the test :(

Pushed by pastithas@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2ae8554e0989 Don't autofocus buttons when about:certerror is embedded in an iframe . r=nhnt11

Thanks for the fix, Panos! Please request Aurora/Beta approval on this when you get a chance.

Comment on attachment 8834911 [details] Don't autofocus buttons when about:certerror is embedded in an iframe (bug 1336352). Approval Request Comment [Feature/Bug causing the regression]: bug 1207107 [User impact if declined]: when a page contains an iframe coming from a broken HTTPS connection, the error page displayed there will steal the focus from the top-level page. This is annoying for people who use the keyboard a lot and likely also impacts a11y. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: only in a local build off m-c so far, Nightly builds with this fix aren't available yet [Needs manual test from QE? If yes, steps to reproduce]: not necessary [List of other uplifts needed for the feature/fix]: no [Is the change risky?]: not really [Why is the change risky/not risky?]: the change only modifies the button focus of the error page, not page structure or appearance that could render it unusable [String changes made/needed]: none

> [Has the fix been verified in Nightly?]: only in a local build off m-c so > far, Nightly builds with this fix aren't available yet I just received a Nightly update and indeed the problem is now fixed.

Marking verified in trunk. Thanks Panos.

Comment on attachment 8834911 [details] Don't autofocus buttons when about:certerror is embedded in an iframe (bug 1336352). focus-related regression fix for ssl error page, beta52+, aurora53+