Last Comment Bug 748803 - Frame with elements blocked through hosts file prevents from scrolling down the page with spacebar
: Frame with elements blocked through hosts file prevents from scrolling down t...
Status: RESOLVED FIXED
[qa+]
: regression, uiwanted
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: 15 Branch
: x86_64 Windows 7
: -- normal with 2 votes (vote)
: mozilla15
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
: 748787 749969 751297 756368 (view as bug list)
Depends on:
Blocks: 749218 301471
  Show dependency treegraph
 
Reported: 2012-04-25 08:49 PDT by Loic
Modified: 2012-07-18 09:23 PDT (History)
15 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
+
verified


Attachments
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page. (3.43 KB, patch)
2012-04-27 11:54 PDT, Boris Zbarsky [:bz]
mounir: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Loic 2012-04-25 08:49:37 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20120425 Firefox/15.0a1
Build ID: 20120425030647

Steps to reproduce:

I think it's a regression. If an element contained in a frame (like ads) is blocked via the hosts file in Windows, it's not possible to use the spacebar to scroll down the page because the focus is stolen by the frame.

STR:
1. Be sure you test with a profile without ad/script blockers like Adblock Plus or NoScript.
2. In Windows, open (as admin) the hosts file (C:\WINDOWS\system32\drivers\etc\hosts)
3. Add the line "127.0.0.1    ad.doubleclick.net" (without " " and with a tab between IP and domain)
4. Open the page http://www.washingtonpost.com/world/africa/sudanese-warplanes-bomb-town-in-south-sudan/2012/04/23/gIQArKp0bT_story.html
NB: the page needs 5-10 sec to be fully loaded (3 loads)
5. Press spacebar to scroll down the page 


Actual results:

Spacebar scrolls down the upper right frame containing the element (an ad) blocked by the hosts file.


Expected results:

Spacebar should scroll down the entire page but not the frame with blocked ad.
Comment 2 Alice0775 White 2012-04-25 21:20:52 PDT
Regression window:
Cannot reproduce
http://hg.mozilla.org/integration/mozilla-inbound/rev/49c9b4e6325b
Mozilla/5.0 (Windows NT 5.1; rv:12.0a1) Gecko/20120124 Firefox/12.0a1 ID:20120124161851
Can reproduce:
http://hg.mozilla.org/integration/mozilla-inbound/rev/f9b12afb9724
Mozilla/5.0 (Windows NT 5.1; rv:12.0a1) Gecko/20120124 Firefox/12.0a1 ID:20120124163248
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=49c9b4e6325b&tochange=f9b12afb9724

Suspected:
b9fe3e1419c6	Wes Johnston — Bug 301471 - Autofocus the Try Again button in the net error dialog. r=bz
Comment 3 Boris Zbarsky [:bz] 2012-04-25 21:47:17 PDT
Hmm.  I wonder whether it would make sense to not autofocus in subframes.  uxwanted!
Comment 4 Boris Zbarsky [:bz] 2012-04-27 00:39:38 PDT
Given bug 749218 I think we should track this and strongly consider backing out bug 301471 if we can't avoid autofocusing in subframes.
Comment 5 Boris Zbarsky [:bz] 2012-04-27 00:41:09 PDT
Mounir, is there a sane way to make autofocus not take effect in a subframe?  Would script that runs in that document before onload and removes the attribute when framed work?
Comment 6 Boris Zbarsky [:bz] 2012-04-27 00:41:38 PDT
Or should autofocus perhaps not do anything in subframes in general?
Comment 7 Mounir Lamouri (:mounir) 2012-04-27 02:03:46 PDT
(In reply to Boris Zbarsky (:bz) from comment #6)
> Or should autofocus perhaps not do anything in subframes in general?

I do not think that would be a good idea. A lot of webpages are doing heavy use of iframe's AFAIK and making autofocus not working in that case would be a very strong limitation. FWIW, Webkit and Presto have that same behavior.

I think a cleaner solution would be to change that error page autofocus behavior and not autofocus if the error page is loaded inside an iframe.
Comment 8 Boris Zbarsky [:bz] 2012-04-27 11:54:42 PDT
Created attachment 619131 [details] [diff] [review]
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page.
Comment 9 :Ehsan Akhgari (away Aug 1-5) 2012-04-27 12:16:32 PDT
Comment on attachment 619131 [details] [diff] [review]
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page.

The overflow properties in the test looks like a mistake!
Comment 10 Boris Zbarsky [:bz] 2012-04-27 12:35:05 PDT
Yes, indeed.  I'll remove those.
Comment 11 Alice0775 White 2012-04-29 05:14:21 PDT
*** Bug 749969 has been marked as a duplicate of this bug. ***
Comment 12 Mounir Lamouri (:mounir) 2012-04-29 14:31:21 PDT
Comment on attachment 619131 [details] [diff] [review]
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page.

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

I think using @autofocus instead of .focus() here is really a detail. I would be okay with .focus() FWIW.

This said, I"m not sure I understand what you are trying to do with the test. I must be missing something but wouldn't have been easier to have a mochitest checking that the focus wasn't on the iframe but on the top-level document after the page is loaded?
Comment 13 Boris Zbarsky [:bz] 2012-04-29 18:14:58 PDT
> I would be okay with .focus() FWIW.

I would rather not, because that doesn't play as well with explicit focus changes by the user.

> This said, I"m not sure I understand what you are trying to do with the test.

What I'm trying to do with this test is basically write a reftest for bug 749218.  Seems to work.  ;)

> wouldn't have been easier to have a mochitest checking that the focus wasn't on the
> iframe

Maybe, if I knew offhand how to check where the focus is.  ;)
Comment 14 Alex Keybl [:akeybl] 2012-04-30 15:31:18 PDT
Do we expect this bug to rear it's head in any other way? Loss of spacebar scrolling when the hosts file has been edited to block certain content seems very edge-casey.
Comment 15 Loic 2012-04-30 15:54:49 PDT
(In reply to Alex Keybl [:akeybl] from comment #14)
> Do we expect this bug to rear it's head in any other way? Loss of spacebar
> scrolling when the hosts file has been edited to block certain content seems
> very edge-casey.

For the record, another bugs similar to this bug have been reported, but with various behaviours. See:
- Bug 748787
- Bug 749218
- Bug 749969
Comment 16 Alex Keybl [:akeybl] 2012-04-30 17:07:24 PDT
(In reply to Loic from comment #15)
> For the record, another bugs similar to this bug have been reported, but
> with various behaviours. See:
> - Bug 748787
> - Bug 749218
> - Bug 749969

Yeah the number of unique instances of this after FF12's release makes this a higher priority for the next release. Let's try to fix this for FF13 if deemed low risk.
Comment 18 Boris Zbarsky [:bz] 2012-04-30 19:33:22 PDT
Comment on attachment 619131 [details] [diff] [review]
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page.

[Approval Request Comment]
Regression caused by (bug #): bug 301471
User impact if declined: Some websites randomly scroll to weird places,
   especially when ad-blockers are in use.  Some websites have odd focus
   behavior, especially when ad-blockers are in use.
Testing completed (on m-c, etc.): Manual verification that both the bad behaviors
   are gone and that full-page error page still fous the "Try Again" button.
   Plus the reftest in the patch.
Risk to taking this patch (and alternatives if risky):  I believe this is
   low-risk.  The error page already depends on script running in it, as far as
   I can tell, so there should be no new issues with users who have script
   disabled: either it already works and will continue to work, or it's already
   completely broken.
String changes made by this patch:  None.
Comment 19 dreapadoir 2012-05-01 14:43:21 PDT
--- Comment #5 from MozillaUser233 <mozillauser233bugzilla@gmail.com> 2012-05-01 14:36:21 PDT ---
(In reply to dreapadoir from comment #4)
> FF12 auto-scrolls down to a section where an add has been blocked.
> This behavior did not occur in previous versions.
> Very annoying and unproductive.
> Is there an estimated time for a fix release?

They say this is the same bug
https://bugzilla.mozilla.org/show_bug.cgi?id=748803

You might get a quicker response from there.
Comment 20 :Ehsan Akhgari (away Aug 1-5) 2012-05-02 21:23:33 PDT
https://hg.mozilla.org/mozilla-central/rev/80ece0284245
Comment 21 Mounir Lamouri (:mounir) 2012-05-03 02:42:15 PDT
(In reply to Boris Zbarsky (:bz) from comment #13)
> Maybe, if I knew offhand how to check where the focus is.  ;)

document.activeElement

Given that the focus for autofocus is made asynchronously, I'm not sure that the test couldn't produce random-green because the snapshot would have been taken before the focus change.
I would have preferred to check for the focused element in the load event after a setTimeout(, 0).

(sorry for writing that comment after the landing, I forgot to reply yesterday)
Comment 22 Loic 2012-05-03 05:38:01 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #20)
> https://hg.mozilla.org/mozilla-central/rev/80ece0284245

This patch in Nightly doesn't seem to fix all the regressions, see my comment https://bugzilla.mozilla.org/show_bug.cgi?id=748787#c13
Comment 23 Alex Keybl [:akeybl] 2012-05-03 09:00:30 PDT
Comment on attachment 619131 [details] [diff] [review]
Don't autofocus the Try Again button in error pages if the error page is not a toplevel page.

[Triage Comment]
Approved for Beta 13 and Aurora 14 given the low risk evaluation and the increasing reports of regression after FF12's release.
Comment 24 Boris Zbarsky [:bz] 2012-05-03 13:58:17 PDT
> Given that the focus for autofocus is made asynchronously

So is the snapshot made onload, no?  I don't think this can accidentally go green.

I would be happy to add an additional mochitest though, if you prefer.
Comment 26 Boris Zbarsky [:bz] 2012-05-03 14:48:23 PDT
*** Bug 748787 has been marked as a duplicate of this bug. ***
Comment 27 Ioana (away) 2012-05-11 06:51:05 PDT
Verified as fixed with the STR from comment 0 on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
BuildID: 20120509070325
Comment 28 Alice0775 White 2012-05-18 02:11:25 PDT
*** Bug 756368 has been marked as a duplicate of this bug. ***
Comment 29 Boris Zbarsky [:bz] 2012-05-21 05:35:53 PDT
*** Bug 751297 has been marked as a duplicate of this bug. ***
Comment 30 Ioana (away) 2012-06-06 07:47:54 PDT
Verified as fixed with the STR from comment 0 on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0 (20120605113340)
Comment 31 Ioana (away) 2012-07-18 09:23:05 PDT
Also verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/20100101 Firefox/15.0

Note You need to log in before you can comment on or make changes to this bug.