Compare bug 105257, "Image can't be changed in JS. Recent regression", although that one is marked Verified Fixed... cc'ing Boris -
I checked the page on several other machines using Mozilla .99. The page worked fine on the initial load (although it did not on my own computer) and only displayed problems when I swapped to another image before the previous image has been displayed. (For example, after the page loads I activated image swap A, but before image swap A's image had completely displayed I activated image swap B. After displaying B image swap A would no longer work. Any time I activated another swap before the previous one had displayed the link ceased to function. When I exited the page, cleared the cache, and reloaded the page everything seemed to work again (until I duplicated the problem). Also, if I tried to swap to an image that had not yet been preloaded it would do nothing and would continue to fail until cache had been cleared and the page reloaded. This is a little different than the problem on my production computer (links just not working on initial load) but I thought I would share it anyway just in case I stumbled across something important.
The other computers I tested on were running Mac OS 9.
"mass" re-assigning of bugs from pav to myself
I am guessing this problem is NOT fixed, however just getting to this I noticed that the page is no longer available. Can you retry with Mozilla 1.3b and if it is still happening, put the site/testcase back up (or attach it to this bug)? till then I am going to mark this as invalid, when the testcase is available, just re-open.
Oh, sorry about that guys! I took everything down when I moved the site quite some time ago. I have posted the test case again with the required script and images. The rest of the page images are missing, but it works in the other browsers again. I will download and test in Mozilla 1.3b tonight.
btw in looking at this using mozilla 1.3b on win2k I am seeing the problem HOWEVER if I do a <shift> mouse-click it seems to load fine. What does the <shift> give me? refetching the image?
<shift> makes us not try to follow the link (since shift-click is "save link").
INTERESTING... this site preloads all of the images using newImage but then later does a document[changeImages.arguments[i]].src = changeImages.arguments[i+1]; This quickly fills up the memory cache (i.e. about:cache?device=memory tells me: Maximum storage size: 4194304 Bytes Storage in use: 6058288 Bytes To get around this... you can do about:config find modify browser.cache.memory.capacity click on the "value" field (i think default is 4096) and change it to 35000. I found that the site when everything is loaded requires 33709k NOTE: while the jpegs on the page are only 24k (or so) we decode them and store these decoded images in memory so with only 4k we overflow it very quickly and then later when the imgCache goes to "get" them... nothing is there.
shouldn't the disk cache be used in that case (and re-decode the image of course)?
I assume that is how it is suppose to work, but something must be wrong. Am starting to go through this now, figuring out when/where decoded bits get written in, what checks get done, what happens when cache is full... I think this will fix a bunch of open bugs.
Um... how is loading behavior in any way, shape or form JS engine? I'd think that would be docshell, if anything...
Following Boris' suggestion and reassigning to Docshell for a look at the issue raised in Comment #6, Comment #11, and Comment #17. The original problem seems to be fixed.
Phil, the original problem is *not* fixed. In fact, it is worse than the reporter realized, because actually both onClick and onMouseOver exhibit the same problem, if you click on a link before the images are cached. See the simplified test case Darin just posted for the 'real' problem.
bz: yeah, i would think the link click would stop any pending network requests, including those for subframes, etc. brendan, jst: i'm out of my element here. please see bz's comment #23 and my simplified testcase in comment #21. thx!!
Well, if it's a widely used idiom, that makes it harder to just mark this INVALID or WONTFIX. What should the precedence be between the href and handler?
This appears to create a problem for Netscape Radio. See internal reference: http://bugscape.nscp.aoltw.net/show_bug.cgi?id=22057
Created attachment 122308 [details] [diff] [review] Patch Is there anything wrong with just doing this? Patch also removes some #include crap from nsWebShell.cpp
Comment on attachment 122308 [details] [diff] [review] Patch This is no good. This would not fix http://bugscape.nscp.aoltw.net/show_bug.cgi?id=22057 , for example.
The patch is not meant to be a thing of beauty but just to simply catch the most common instance of this problem. The alternative is a far more extensive rewiring of docshell so that it doesn't stop requests when clicking on a js link. I'm not convinced that won't introduce its own problems.
How hard is it to change things so that docshell evaluates the JS synchronously? /be
ADT: Nominating topembed
Do we execute JS in data: URLs? I (naively) expected not, because we don't seem to execute JS that's loaded as a top-level document via an http: or file: URL.
as for data: URLs... wouldn't a data: URL always result in a new document? in which case it seems appropriate for a link click to a data: URL to stop any and all processing in the docshell.
What about wacky things such as js that changes window.location.href to point to more js and things of that nature?
Ah, correct you are (Mike and Darin). As for JS that changes the location or document.writes() or anything like that, all that should be fine. If the location is changed, or if document.write() is called, the code that runs as a result of that stops all network traffic, and if none of that happens and it doesn't return any data, we won't cancel, and that's what we want. And some data is returned, then we'll stop whatever happened until that point. Also what we want.
I think we want this for 1.4final, esp. if it's nsbeta1+ -- and whether or not its patch fixes a regression from 1.0. /be
Reassigning to Johnny since he's actively working on this.
*** Bug 192923 has been marked as a duplicate of this bug. ***
XPCOM doesn't explict state either, but rather XPCOM lets this question be answered by the interface contract. (1) is the most common practice. |SchemeIs| does not define any specific error codes. It is possible that the contract intended requires a boolean result. Although I think that any XPCOM method should be able to return an XPCOM error code without the interface explictly mentioning these codes.
Are the likes of mailto: also affected by this problem?
Comment on attachment 123145 [details] [diff] [review] Same thing, but don't add nsJSChannel to the load group. r=adamlock I'm not sure I like the schemeisrv being declared in the #ifdef DEBUG and used outside of it as it is. Is SchemesIs() likely to fail anyhow?
Comment on attachment 123145 [details] [diff] [review] Same thing, but don't add nsJSChannel to the load group. shaver made a good point (via email) that functions really shouldn't set out params unless they are going to succeed. otherwise we'd have problems in the case of |out nsIFoo|. of course, even that rule isn't strictly followed in the codebase. i guess it would be nice to say that there is a hard and fast rule to follow in these cases, but it seems like it just comes down to the specifics of particular methods. in this case, we find it pathological for SchemeIs to fail and set its out param to TRUE, so let's just keep things simple... i vote for jst's original code... sorry for all the useless raucous ;-)
darin: you have shamed me for my closed-system thinking ;-). I agree, test rv and interpret the out param only on success. bz (re: comment 56): don't be callin' my baby ugly now! /be
Darn, I crossed streams with shaver -- armageddon is upon us for sure. I'm indifferent. Shaver's right about interface pointer out params, but boolean and other scalars could be junk, in a pathological case (like, every other plugin we've had to work around). I defer to jst. /be
I'll return early, given that this may have security consequences. If someone thinks they have reasons to change this later, feel free to do so, but don't bother asking for my oppinion :-)
Comment on attachment 123145 [details] [diff] [review] Same thing, but don't add nsJSChannel to the load group. Requesting approval for 1.4final.
Comment on attachment 123145 [details] [diff] [review] Same thing, but don't add nsJSChannel to the load group. a=asa (on behalf of drivers) for checkin to 1.4
The given URL seems to work fine for me using today's Mozilla trunk binary 2003051508 on WinNT. James: does this work OK for you now, too?
*** Bug 144587 has been marked as a duplicate of this bug. ***
*** Bug 112398 has been marked as a duplicate of this bug. ***