Closed
Bug 313337
Opened 19 years ago
Closed 18 years ago
Fastclick popup gets around popup blocking by abusing the onchange event
Categories
(Core :: Layout: Form Controls, defect)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla1.8.1beta2
People
(Reporter: jruderman, Assigned: jst)
References
()
Details
(Keywords: fixed1.8.1, testcase)
Attachments
(2 files, 4 obsolete files)
515 bytes,
text/html
|
Details | |
2.17 KB,
patch
|
bzbarsky
:
review+
sicking
:
superreview+
mtschrep
:
approval1.8.1+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.8rc1?
Reporter | ||
Comment 1•19 years ago
|
||
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
Reporter | ||
Comment 2•19 years ago
|
||
Reporter | ||
Updated•19 years ago
|
Reporter | ||
Updated•19 years ago
|
Summary: Fastclick popup gets around popup blocking on Firefox 1.5 branch → Fastclick popup gets around popup blocking by abusing the onchange event
Reporter | ||
Comment 5•19 years ago
|
||
IE doesn't fire onchange in this case, because the value of the textbox was set by script. Firefox probably shouldn't either.
Comment 6•19 years ago
|
||
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+
Reporter | ||
Comment 7•19 years ago
|
||
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.
Reporter | ||
Updated•19 years ago
|
Attachment #200483 -
Flags: superreview?(bzbarsky)
Attachment #200483 -
Flags: review?(bryner)
Reporter | ||
Updated•19 years ago
|
Severity: normal → major
Component: Plug-ins → Layout: Form Controls
QA Contact: plugins → layout.form-controls
Reporter | ||
Updated•19 years ago
|
Assignee: nobody → jruderman
Reporter | ||
Updated•19 years ago
|
OS: MacOS X → All
Hardware: Macintosh → All
![]() |
||
Comment 8•19 years ago
|
||
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.
Comment 9•19 years ago
|
||
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?
Reporter | ||
Comment 10•19 years ago
|
||
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
Comment 11•19 years ago
|
||
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).
Reporter | ||
Comment 12•19 years ago
|
||
If you open open the testcase in a background tab, it won't work until you reload due to bug 307933.
Reporter | ||
Comment 13•19 years ago
|
||
bz says messing with focus and onchange this close to Firefox 1.5 would be risky.
Updated•19 years ago
|
Flags: blocking1.8rc1? → blocking1.8rc1-
![]() |
||
Updated•19 years ago
|
Attachment #200483 -
Flags: superreview?(bzbarsky) → superreview-
Reporter | ||
Updated•19 years ago
|
Flags: blocking-aviary2?
Reporter | ||
Updated•19 years ago
|
Blocks: fastclick-popup
Updated•19 years ago
|
Flags: blocking-aviary2? → blocking1.8.1?
Updated•19 years ago
|
Assignee: nobody → jst
Assignee | ||
Comment 16•19 years ago
|
||
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)
Assignee | ||
Updated•19 years ago
|
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)
Assignee | ||
Comment 17•19 years ago
|
||
Right diff this time.
Attachment #227336 -
Flags: superreview?(bugmail)
Attachment #227336 -
Flags: review?(bzbarsky)
![]() |
||
Updated•19 years ago
|
Attachment #227336 -
Flags: review?(bzbarsky) → review+
Attachment #227336 -
Flags: superreview?(bugmail) → superreview+
Sorry about the late review, it got burried in my review queue. sr=sicking
Assignee | ||
Updated•18 years ago
|
Attachment #200483 -
Attachment is obsolete: true
Attachment #200483 -
Flags: review?(bryner)
Assignee | ||
Updated•18 years ago
|
Attachment #227336 -
Flags: approval1.8.1?
Assignee | ||
Comment 20•18 years ago
|
||
Fix landed on the trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment 21•18 years ago
|
||
Pushing this out to beta2 so it can bake on the trunk a little more.
Target Milestone: mozilla1.8.1beta1 → mozilla1.8.1beta2
Updated•18 years ago
|
Attachment #227336 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Updated•18 years ago
|
Keywords: fixed1.8.1
Comment 22•18 years ago
|
||
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
Comment 25•18 years ago
|
||
(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
You need to log in
before you can comment on or make changes to this bug.
Description
•