Closed Bug 313337 Opened 15 years ago Closed 15 years ago

Fastclick popup gets around popup blocking by abusing the onchange event

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla1.8.1beta2

People

(Reporter: jruderman, Assigned: jst)

References

()

Details

(Keywords: fixed1.8.1, testcase)

Attachments

(2 files, 4 obsolete files)

From bug 176079.  I was able to reproduce once on Mac.


------- Bug 176079 comment 89 From David Barr -------

Ucomics has figured out a way around the fix...
http://www.ucomics.com/forbetterorforworse/2005/10/11/


------- Bug 176079 comment 90 From Bob Clary -------

This appears to be a fastclick "innovation". It is opening popunder windows on
page unload using flash. I also see this on http://wsls.com/ which is also
fastclick related. See this in today's ff 1.8 branch build. And yes, I do have
privacy.popups.disable_from_plugins set to 2.
Flags: blocking1.8rc1?
I'm not convinced this is Flash-related based on my partially simplified
testcase.  It's possible that Bob and I are seeing different exploits, though.
Summary: Fastclick popup gets around Flash popup blocking on Firefox 1.5 branch → Fastclick popup gets around popup blocking on Firefox 1.5 branch
Attached file partially simplified testcase (obsolete) —
Attached file a little simpler (obsolete) —
Attachment #200411 - Attachment is obsolete: true
Attached file simplified testcase
Attachment #200415 - Attachment is obsolete: true
Keywords: qawantedtestcase
Summary: Fastclick popup gets around popup blocking on Firefox 1.5 branch → Fastclick popup gets around popup blocking by abusing the onchange event
IE doesn't fire onchange in this case, because the value of the textbox was set
by script.  Firefox probably shouldn't either.
plussing this. I think this might be the last major case where pop-ups are
getting around our blocker. We may not get this but I don't want it to fall off
our radar.
Flags: blocking1.8rc1? → blocking1.8rc1+
Attached patch patch (obsolete) — Splinter Review
Remaining issues:

* I'm not sure where in SetValue this should go.  For example, should it be inside the big |if| or outside of it?

* This causes onchange to not fire if you use autocomplete and then un-focus the textbox, probably for the same reason using autocomplete doesn't fire an oninput event.
Attachment #200483 - Flags: superreview?(bzbarsky)
Attachment #200483 - Flags: review?(bryner)
Severity: normal → major
Component: Plug-ins → Layout: Form Controls
QA Contact: plugins → layout.form-controls
Assignee: nobody → jruderman
OS: MacOS X → All
Hardware: Macintosh → All
Comment on attachment 200483 [details] [diff] [review]
patch

Please use the -p flag for diffs; makes it a lot easier to see what's going on...

First of all, this seems like it would cause a pretty nontrivial a performance issue.  InitFocusedValue is slow.  Very slow, for anything nontrivial (try this very textarea with a comment that has some lines in it).  Given all the sites that set values on text controls that users don't actually interact with, we probably should not be running InitFocusedValue() unless we're in the unlikely case of this bug -- setting the value on a text control that currently has focus.

Also, breaking autocomplete firing onchange is no good.  That should be fixed before we take this patch.
We're very short on time for this release and we'd need a very safe fix to consider for 1.5. Setting blocking flag back to ?
Flags: blocking1.8rc1+ → blocking1.8rc1?
I think the problem with form autocomplete is that autocomplete uses nsFormFillController::SetTextValue and nsFormFillController::SetTextValue calls SetValue directly.  I don't know the right way to fix that and I don't know whether other autocomplete (e.g. address bar) is broken in a similar way.

bz suggested setting mFocusedValue directly instead of calling InitFocusedValue.  That still does a copy but it at least avoids serializing a DOM.
Assignee: jruderman → nobody
I'm not getting popups from these testcases. I do from the original page when I click on a link, but that's a different issue (they've got an onclick handler).
If you open open the testcase in a background tab, it won't work until you reload due to bug 307933.
bz says messing with focus and onchange this close to Firefox 1.5 would be risky.
Flags: blocking1.8rc1? → blocking1.8rc1-
Attachment #200483 - Flags: superreview?(bzbarsky) → superreview-
Flags: blocking-aviary2?
Flags: blocking-aviary2? → blocking1.8.1?
*** Bug 326860 has been marked as a duplicate of this bug. ***
JST - any thoughts on this?
Flags: blocking1.8.1? → blocking1.8.1+
Assignee: nobody → jst
Attached patch Wrong diff... (obsolete) — Splinter Review
This is the earlier patch for this bug with bz's suggestions in comment #8.
Attachment #227335 - Flags: superreview?(bugmail)
Attachment #227335 - Flags: review?(bzbarsky)
Attachment #227335 - Attachment description: Updated patch based on bz's comments. → Wrong diff...
Attachment #227335 - Attachment is obsolete: true
Attachment #227335 - Flags: superreview?(bugmail)
Attachment #227335 - Flags: review?(bzbarsky)
Right diff this time.
Attachment #227336 - Flags: superreview?(bugmail)
Attachment #227336 - Flags: review?(bzbarsky)
Attachment #227336 - Flags: review?(bzbarsky) → review+
Sicking: review this puppy!
Target Milestone: --- → mozilla1.8.1beta1
Attachment #227336 - Flags: superreview?(bugmail) → superreview+
Sorry about the late review, it got burried in my review queue.

sr=sicking
Attachment #200483 - Attachment is obsolete: true
Attachment #200483 - Flags: review?(bryner)
Attachment #227336 - Flags: approval1.8.1?
Fix landed on the trunk.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Pushing this out to beta2 so it can bake on the trunk a little more.
Target Milestone: mozilla1.8.1beta1 → mozilla1.8.1beta2
Attachment #227336 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
Still get the random Fastclick.net pop-up browsing http://www.screamscape.com/ using the latest branch build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060714 BonEcho/2.0b1 ID:2006071414

~B
*** Bug 344858 has been marked as a duplicate of this bug. ***
Depends on: 355362
jst, was this checked into 1.8?
(In reply to comment #24)
> jst, was this checked into 1.8?
> 

Bob, I'm not sure as I still see this at a number of sites.

~B
Depends on: 357684
Depends on: 355367
Depends on: 370521
You need to log in before you can comment on or make changes to this bug.