.blur() results in window being lowered and some other window being raised (regardless of value of dom.disable_window_flip) (popunders are possible)

RESOLVED FIXED in mozilla2.0b7



12 years ago
a month ago


(Reporter: csthomas, Assigned: mounir)


(Blocks 1 bug)

Dependency tree / graph
Bug Flags:
wanted1.9.0.x -
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+, status1.9.2 wanted)




(3 attachments, 3 obsolete attachments)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070111 SeaMonkey/1.1 Mnenhy/

Paste the URL in the URL bar, and open it in a new tab (ctrl+[shift+]enter).  Make sure you have another window open from the same process (e.g. chatzilla, mailnews, JS console).
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv: Gecko/20070111 SeaMonkey/1.1

When I just plain click on the URL in this bug, I get an alert:

The URL field currently contains a URL using either a javascript or data URI scheme, which means your Bugzilla session is vulnerable to various types of attack if you click the link. The field was last changed by Chris Thomas (CTho) [formerly cst@andrew.cmu.edu] <cst@yecc.com>.   ...

When I click on "proceed", I wind up in the Chatzilla window I had open with the "asdf" window underneath.   
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
<gavin> looks to me like its just a matter of adding the pref-checking logic in nsGlobalWindow::Focus to nsGlobalWindow::Blur
Assignee: general → general
Component: General → DOM
Product: Mozilla Application Suite → Core
QA Contact: general → ian
Duplicate of this bug: 369854

Comment 4

12 years ago
I am not sure comment #2 is correct, as a call to self.focus() *also* ignores the value of dom.disable_window_flip and raises the window.  This is the source of the recently renewed plague of popunder ads (for example http://www.trafficmasterz.net/test_popunder.php, though their source is childishly obfuscated, uses self.focus() to implement the popunder).  And Firefox had come so close to eliminating them forever...

Comment 5

12 years ago
Posted file popunder testcase (obsolete) —
illustrates that blur() and self.focus() work despite dom.disable_window_flip, and allow the creation of evil popunders.

Comment 6

12 years ago
I can reproduce with James's testcase.  This isn't too surprising; see bug 355482 comment 12.


12 years ago
Summary: .blur() results in window being lowered and some other window being raised (regardless of value of dom.disable_window_flip) → .blur() results in window being lowered and some other window being raised (regardless of value of dom.disable_window_flip) (popunders are possible)

Comment 7

11 years ago
Posted patch proposed patch (obsolete) — Splinter Review
This patch adds checking to nsGlobalWindow::Blur() as proposed in comment 2, and also adds an extra check to nsGlobalWindow::Focus() which only allows documents to focus windows they have opened.  I added a new method nsContentUtils::GetWindowFromCaller() in support of this.  

This patch fixes the testcase.  This is my first Mozilla contribution, so I'm definitely open to any feedback about better ways to do stuff, or things I've done wrong.

Comment 8

11 years ago
Attachment #287983 - Attachment is obsolete: true


11 years ago
Attachment #296122 - Flags: review?(jst)


11 years ago
Flags: wanted1.9.0.x?
Right now, this bug looks like it only applies to 1.8, so not wanted for 1.9.0.x. If that's not the case, please re-nom.
Flags: wanted1.9.0.x? → wanted1.9.0.x-
Whiteboard: [needs r jst]

Comment 10

11 years ago
I'm not sure what you mean; this bug is reproducible in the latest nightly build with the attached testcase.  I can't re-nominate this bug myself.


11 years ago
Flags: wanted1.9.0.x- → wanted1.9.0.x?
Doesn't really meet the "wanted" criteria (security, stability, regression from maintenance release) for 1.9.0.x. However, we'll look at a reviewed and baked patch.
Flags: wanted1.9.0.x? → wanted1.9.0.x-
Version: 1.8 Branch → Trunk

Comment 12

10 years ago
This is indeed a regression, and it seems like the regression itself was introduced in a maintenance release (

Seems as if you can't even use CAPS to block access to Window.blur for some reason.  Is there any way to make an extension out of the patch until it gets checked in?


10 years ago
Blocks: popups
Assignee: general → nobody
QA Contact: ian → general
Comment on attachment 296122 [details] [diff] [review]
proposed patch

Is this still an issue? Loading the testcase it seems like all tests produce popups, not popunders, which I believe is what we want here. Clearing review flag (which got lost and was lost for way too long, my apologies). Please re-request reviews if we still need this.
Attachment #296122 - Flags: review?(jst)

Comment 14

9 years ago
This is still an issue.  Using the latest Fx nightly build all the testcases produce popunders for me (on Windows).  

I don't know how to re-request review on this; I'm not sure I have the bugzilla permissions for that.
Duplicate of this bug: 545337
We should get this fix in for 1.9.3. And as part of the fix for this, we need automated tests that check that this doesn't regress, and one part we should do in that testing is to see what happens if we not only focus a window, but the document or an element inside that window to make sure that works as expected as well.
blocking2.0: --- → beta1
Comment on attachment 296122 [details] [diff] [review]
proposed patch

should jst review then?
Attachment #296122 - Flags: review?(jst)

Comment 18

9 years ago
Comment on attachment 296122 [details] [diff] [review]
proposed patch

An updated patch is needed here.
Attachment #296122 - Flags: review?(jst)
blocking2.0: beta1+ → beta2+
(In reply to comment #8)
> Created an attachment (id=296123) [details]
> more extensive testcase

Interestingly, I get popups on Linux.  And popunders on Windows.
Assignee: nobody → jst
blocking2.0: beta2+ → beta3+
Do we expect this to hit code freeze? Marked beta3+ 10 days ago, no activity since then.

Enn: what sort of update to the patch is needed?

Comment 21

9 years ago
(In reply to comment #20)
> Enn: what sort of update to the patch is needed?

The current patch in the bug is over two years old. The approach seems fine though.
jst, can you help us get the patch unbitrotted?
betaN+, not critical for beta3.
blocking2.0: beta3+ → betaN+

Comment 24

9 years ago
Posted patch Refreshed patch (obsolete) — Splinter Review
The patch is now refreshed. I don't know if that's working because the bug can't be reproduced on GNU/Linux systems. I 'm going to send it to the try server so we will have some builds to test it.
Attachment #296122 - Attachment is obsolete: true
Attachment #464868 - Flags: review?(jst)


9 years ago
Whiteboard: [needs r jst]

Comment 26

9 years ago
I will work on that after beta5 (fixing broken tests and writing new ones).
By the way, I tested on MacOS X and the patch is working.
Assignee: jst → mounir.lamouri
Comment on attachment 472866 [details] [diff] [review]
Part 1 - If dom.disable_window_flip is enable, block .blur() and .focus() if opener is different from the caller

+nsPIDOMWindow *
+  JSContext *cx = nsnull;
+  sThreadJSContextStack->Peek(&cx);
+  if (cx) {
+    nsCOMPtr<nsPIDOMWindow> win =
+      do_QueryInterface(nsJSUtils::GetDynamicScriptGlobal(cx));
+    return win;
+  }
+  return nsnull;

This is effectively the same code as GetDocumentFromContext(), but it should really be the same as GetDocumentFromCaller(), which return the correct window in more cases than the above code. How about we rename GetDocumentFromCaller() to GetWindowFromCaller() and make a new GetDocumentFromCaller() function that calls GetWindowFromCaller() and then calls GetExtantWindow() on the returned document, assuming it's not null?

This patch looks good otherwise, but r- until the above is dealt with.
Attachment #472866 - Flags: review?(jst) → review-

Comment 30

9 years ago
Comment on attachment 472866 [details] [diff] [review]
Part 1 - If dom.disable_window_flip is enable, block .blur() and .focus() if opener is different from the caller

It looks like GetDocumentFromCaller isn't really doing what we were expecting.
Re-asking a review then.
Attachment #472866 - Flags: review- → review?(jst)
Comment on attachment 472866 [details] [diff] [review]
Part 1 - If dom.disable_window_flip is enable, block .blur() and .focus() if opener is different from the caller

Yup, GetDocumentFromCaller() isn't what we want here after all.
Attachment #472866 - Flags: review?(jst) → review+
Attachment #472867 - Attachment is patch: true
Attachment #472867 - Attachment mime type: application/octet-stream → text/plain
Attachment #472867 - Flags: review?(jst) → review+

Comment 32

9 years ago

Thank you for your contribution James :)
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6


9 years ago
Blocks: 502824
Depends on: 595100


9 years ago
Duplicate of this bug: 610562
Duplicate of this bug: 644339
Duplicate of this bug: 505210
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.