Closed
Bug 277092
Opened 20 years ago
Closed 20 years ago
Calls to document.location do not clear interval timers
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta1
People
(Reporter: adamlock, Assigned: bzbarsky)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
JS that changes the location within a setInterval timer can fire more than once.
The interval timer is not cleared even after the location is changed and
continues to fire until content arrives to replace the existing document. The
correct behaviour is probably to clear all interval timers immediately when the
location is changed.
I'll attach a simple test case next.
Steps to reproduce in the real world:
1. Enable popups in the browser (Firefox or Mozilla).
2. Load "http://www.networksolutions.com/en_US/whois/index.jhtml"
3. Browse away to another site, e.g. http://www.mozilla.org
Sometimes the browser will complain that httpshttps is not a valid protocol.
This is due to a time firing more than once (see below).
Analysis:
Basically the site attempts to open this popup.
http://www.networksolutions.com/en_US/popexit/popstealth.html
If you view the source to this html you'll notice that when it loads, it starts
a "checkfx" timer using the JS setInterval() call, running once a second. This
checkfx() call tests the location of the main page to see if you have moved to
another site and then calls an error1() and error2() functions that append
"http" or "https" to the popup's url value to redirect the popup to a site survey.
So the url should expand out to something like this:
https://www.networksolutions.com/en_US/popexit/popstealth2.html?adName=1
The popup's interval timer attempts to load the URL above but before it can
deliver content the timer fires again and appends another https so it becomes:
httpshttps://www.networksolutions.com/en_US/popexit/popstealth2.html?adName=1
At this point you're stuck. The malformed URL generates an error message in
Mozilla, the timer appends another "https" to become httpshttpshttps and you're
caught in an infinite loop forever.
The site should certainly be calling clearInterval() to remove the timer, but it
isn't. Therefore I assume that IE automatically cancels interval timers when the
location is changed but Mozilla doesn't. Perhaps it should, or perhaps it's
undefined behaviour and therefore something the site has to deal with.
If it test works, try flushing the cache first or using a slower loading URL
within the timer.
what if i set my document.location to
"javascript:void(window.document.title='hello world')"
Assignee | ||
Comment 3•20 years ago
|
||
So should STOP_CONTENT stop timers, basically? That would make some sense...
Comment 5•20 years ago
|
||
would that allow pressing stop to stop DHTML animations? sounds good to me
Assignee | ||
Comment 6•20 years ago
|
||
Yes, I suspect that would have the effect of stopping DHTML when "stop" is
clicked (if the page is already loaded and all). You're correct that this seems
reasonable.
Comment 7•20 years ago
|
||
Sounds resonable indeed. If someone fixes this before beta, I think we could
take this. It does need testing in the wild though for possible regressions.
Assignee | ||
Comment 8•20 years ago
|
||
Attachment #172996 -
Flags: superreview?(jst)
Attachment #172996 -
Flags: review?(jst)
Comment 9•20 years ago
|
||
Comment on attachment 172996 [details] [diff] [review]
Proposed patch
Yeah, we should see how this handles the real world sites... r+sr=jst
Attachment #172996 -
Flags: superreview?(jst)
Attachment #172996 -
Flags: superreview+
Attachment #172996 -
Flags: review?(jst)
Attachment #172996 -
Flags: review+
Assignee | ||
Updated•20 years ago
|
Assignee: general → bzbarsky
Target Milestone: --- → mozilla1.8beta
Assignee | ||
Comment 10•20 years ago
|
||
Fixed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 11•20 years ago
|
||
I think this broke Netflix's dynamic star ratings. See bug 292921.
Assignee | ||
Comment 12•20 years ago
|
||
This patch will probably need to be backed out. See bug 292921
Depends on: 292921
You need to log in
before you can comment on or make changes to this bug.
Description
•