68955 - Click-handlers in Links don't change frame or image source

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Wolfgang Schwarz]

It seems to me that Bug 57103, Bug 64349, Bug 64395, Bug 67499, and maybe Bug 
67115, all come down to the same problem. But since this problem isn't stated 
generally at any of these bug reports I decided to file a new one. My apologies 
if I'm wrong and these are really seperate issues.

So here is the problem:

A link's onlick handler cannot change the source of images or (i)frames. The 
images just disappear, the frames remain at the old location.


Examples:

   <a href="one.html" target="otherFrame" 
      onclick="self.location.replace('two.html')">

should change the location of the current frame, but it doesn't. Same with:

   <a href="javascript:void 0" 
      onclick="self.location.replace('two.html')">

Similarly,

   <a href="one.html" target="otherFrame" 
      onclick="document.images[0].src='1.gif'">

should change the source of an image, but instead the image disappears. Same 
with:

   <a href="javascript:void 0" 
      onclick="document.images[0].src='1.gif'">


There are two exceptions: All these operations work correctly if 
   a) the links's href is a local anchor (like "#bla"), or
   b) the onclick handler does return false.

Comment 1

[Wolfgang Schwarz]

oops, sorry. The second bug in that list should be Bug 64394, not 64349.

Comment 2

[Johnny Stenback (:jst)]

Testcase?

Comment 3

[Wolfgang Schwarz]

Attachment: testcase

Comment 4

[Johnny Stenback (:jst)]

Link targetting problems, rpotts is working on this so he'll get this bug :-)

Comment 5

[Fabian Guisset]

hmm thanks for doing this Wolfgang. Add bug 56067 to that list. I'm copying
comments from jst on bug 56067 :

"The reason this doesn't work is that loading a javascipt: URL calls StopLoad()
even if evaluation of the javascript: URL doesn't return a value. So here's what
happens when the lower image in the testcase is clicked on:

  - We call the onclick handler on the link, the onclick handler sets the .src
of an image which kicks off a load of that image.

  - The URL of the link is loaded, this is the javascript: URL and now, before
we know that this javascript: URL doesn't return a value we wanto show in the
browser we cancel all loads targetted at the current window, this will stop the
image load we just initiated.

It seems like the whole stopping of loads targetted at the current window should
only happen once new data is on it's way to the new window and not immediately
when someone clicks on a link on the window."

Comment 6

[rpotts (gone)]

This shoud be fixed by the patch attached to bug #65777

Comment 7

[Wolfgang Schwarz]

hm, with build 2001-05-21-04 at least this is only partly fixed: The image in
the testcase still disappears when it should be replaced.

Comment 8

[Prashant Desale]

Wolfgang is right. Bug is not completely fixed. The image in
the testcase still disappears when it should be replaced.
Reopening.

Comment 9

[selmer (gone)]

Is 56067 a dup of this?

How many sites/users are affected by this?  I'm tempted to take it off the 091
radar, but I'll wait for the response to do that.  If this isn't hitting many
folks, please move it off 091.  Otherwise, please explain why we should respin
for this.

Comment 10

[Prashant Desale]

I don't think it affects sites/users, so it can wait. We can surely move it 
forward to 092. 

Comment 11

[rpotts (gone)]

I've tried running the attached test case using the 0.9.1 netscape 6.1 build and
I'm seeing the first 3 tests succeed and the final test fail.

The test that fails is the link Source:<a href="javascript: void 0"
onclick="self.location.replace('http://www.mozillazine.org')">

I believe that this is related to the work that is remaining in bug #65777.  The
issue there is that javascript: URLs are not executed synchronously... 
Therefore there is a race condition between evaluating the "javascript: void 0"
and doing the "self.location.replace(...)".

Is this the behavior that everyone else is seeing?  Or do the other tests fail
in some situations?

- -rick

Comment 12

[Wolfgang Schwarz]

this is strange: I remember that the last time I tried it, the first two tests 
failed, but now they definitely work - with M0.91 and with 2001-05-21-04. 

Today I get exactly what you describe: The first three tests work and the the 
final one fails.

Comment 13

[rpotts (gone)]

Ok..  then if this is the 'new state of the world'... i'll verify that the final 
test is failing because of the work that's left to do in bug #65777...

If that's the case, then i'll feel better - because at least i'll know what's 
going on :-)

-- rick

Comment 14

[rpotts (gone)]

i'm beginning to think that there is a second bug associated with chaging the
src of an image.

it appears that if the '2' image is *not* in the cache, then it is not properly
displayed in the first two tests...  Instead the '1' image is replaced by a
blank image.

however, if the '2' image is explicitly loaded (and thus placed in the image
cache) then the first two tests work fine!

I need to determine if this behavior is due to the loadgroup being cancelled
(and stopping the image load) or if it is someting within imagelib that's going
wrong...

When the image is loaded from the cache, i believe that it occurs
synchronously... so the request would not stay in the loadgroup very long...

Comment 15

[rpotts (gone)]

I've done a bit more investigating and here is what I've discovered...

1. OnClick handlers always fire *before* the link's href is loaded.  This is
true for Nav 4.x, IE 5 and Mozilla.

2. When a href is loaded, the currently loading URIs are cancelled.  This is
also the behavior of Nav 4.x IE 5 and Mozilla.

Keping this in mind, the test cases start to look pretty nasty :-)

Test #1 : <a href="javascript:void 0"
onclick="document.pic.src='http://www.umsu.de/charms/texte/2.gif'">
Will *only* work if the image '2.gif' is already cached.  Otherwise, when the
link is clicked, the '1' will dissappear and an empty box will be drawn.  
This behavior is consistant for Nav 4.x IE 5 and Mozilla.

Test #2 : <a href="http://www.mozilla.org" target="newW"
onclick="document.pic.src='http://www.umsu.de/charms/texte/2.gif'">
Should *always* work.  If fact, after clicking on test #2, test #1 should *also*
start working (because the image has been cached).

Test #3 : <a href="http://www.mozilla.org" target="newW"
onclick="self.location.replace('http://www.mozillazine.org')">
Should *always* work.

Test #4 : <a href="javascript: void 0"
onclick="self.location.replace('http://www.mozillazine.org')">
Should *never* work!  This is because, even though the javascript: URL has no
output, the currently loading URIs (ie mozillazine) will be cancelled *before*
evaluating the JS URL.  This is true for Nav 4.x, IE 5 and Mozilla !!

Comment 16

[Wolfgang Schwarz]

oh dear, sorry for causing so much confusion.

I've also done some tests with other browsers now, and you seem to be quite 
right. The remaining issue is then only that testcase 2 fails when the image is 
not already cached. 

Comment 17

[rpotts (gone)]

ok...  i think that i've finally figured out what is happening with test #2!!

It appears that if, after loading the page, you click on test #2 (without
clicking on test #1 first) everything works fine.  It is only if you click on
test #1 before clicking on test #2 that the '2' image will not appear.

The reason for this is that when test #1 is loaded, the 'src' attribute on the
image is set to '...2.gif'.  Unfortunately, the image is cancelled while it is
being loaded - so it does not appear.  Then when tes #2 is loaded, there is the
following piece of code in nsGenericHTMLElement::SetAttribute(...):

    // don't do any update if old == new
    result = GetAttribute(aNameSpaceID, aAttribute, strValue);
    if ((NS_CONTENT_ATTR_NOT_THERE != result) && aValue.Equals(strValue)) {
      return NS_OK;
    }

Since the 'src' attribute of the image is already set to '...2.gif' (from the
previous test) we return without trying to load the image !!!

It appears that both IE5 and Nav 4.x either realize that the image load failed,
or do not perform this optimization - because they correctly reload '2.gif'.

Clearly, this behavior is not related to window targeting or onClick() handlers
in particular...  It is simply a breakdown in communication between the image
which is being loaded and the HTML Image element/frame...

Comment 18

[Wolfgang Schwarz]

I think you should not perform this check in case of images, frames and the 
like. This can break functionality in many other cases as well, in particular
if you want to reload an image (or whatever) that is dynamically created by the 
server to get the newest version, e.g. for webcams.

I've made another test case for this: 
Go to http://www.umsu.de/tests/phpimg.html and press the link "get new image". 

In NN4 this will send a request for the same image again, usually replacing the 
displayed number by another one (the image URL randomly sends one of 9 different 
images of numbers). In Mozilla this fails.

Comment 19

[rpotts (gone)]

I agree...  

Here's a link to the CVS log message:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/content/html/content/src/nsGenericHTMLElement.cpp#1.176

It appears that this check is an optimization whose side effects were not fully
considered :-)  Is 'src' the only attribute which should *not* be optimized?

Comment 20

[rpotts (gone)]

I forgot to mention that this optimization was added to fix bug #21879.

Comment 21

[rpotts (gone)]

I'm cc'ing vidur and shaver since they added the optimization in the first place...

Comment 22

[Wolfgang Schwarz]

There is more than just SRC, although SRC is certainly the most common 
candidate. Here are some other attributes that fetch something from the server:

HREF (for LINK elements)
DATA
ARCHIVE
BACKGROUND and the CSS correlate style.backgroundImage

Comment 23

[rpotts (gone)]

Attachment: Patch to fix short-circuit optimization for image.src attribute

Comment 24

[rpotts (gone)]

I've just attached at patch to nsHTMLImageElement.cpp which prevents the
short-circuit optimization of the 'src' attribute.  This patch fixes test #2 in
the attached test case.

Unfortunately, it does *not* fix http://www.umsu.de/tests/phpimg.html which
turns out to expose *yet another bug*!!

The remaining problem is with the imagelib cache.  It turns imglib seems to
ignore the caching policy headers returned by the server.  So, if an image is
cached, it is not properly re-validated in the
http://www.umsu.de/tests/phpimg.html case :-(

Comment 25

[rpotts (gone)]

I forgot to mention that the first patch is still important :-)  Because,
without it, you won't even get to the second bug :-)

-- rick

Comment 26

[Johnny Stenback (:jst)]

sr=jst for the attached patch.

Comment 27

[vidur (gone)]

r=vidur

Comment 28

[chris hofmann]

a=chofmann

Comment 29

[selmer (gone)]

Please check in asap.

Comment 30

[rpotts (gone)]

i've just checked this patch into the trunk...

i'll check it into the 0.9.2 branch shortly.

Comment 31

[rpotts (gone)]

i've just checked the patch into the 0.9.2 branch...

Comment 32

[rpotts (gone)]

i'm moving this bug out to the next milestone...

the remaining issues have to do with the imglib cache :-(

Comment 33

[rpotts (gone)]

I've just opened bug #87710 to track the issues with the image cache.

So, since all of the link targeting issues have been resolved, i'm going to
close this one out :-)

-- rick

Comment 34

[rpotts (gone)]

*** Bug 56067 has been marked as a duplicate of this bug. ***

Comment 35

[Prashant Desale]

Hey Rick,
I just tested this one on todays branch [2001-07-23-06] with win-98 & WinNT 
both.

Here is what I observed. If you want to see the failures, please folow the steps 
I say.
STEPS TO FOLLOW:
1] Click on first link. [Result: First Test fails]
2] click on third link. [Result: Third Test pass]
3] Go back to testcase.
4] Click on first link. [Result: First Test fails again]
5] Click on second link. [Result: Second Test pass]
6] Click on first link. [Result: First Test pass]
7] Reload the page. [hold Shift key & click Reload]
8] Click on first link. [Result: First Test pass]

PROBLEM:
Why first test fails in "step-1" & "step-4".

Re-opening bug. Please close it again if I'm wrong somewhere.

Comment 36

[rpotts (gone)]

As i mentioned in a previous comment, i believe that test #1 should only succeed
*after* test #2 has been run.

running test #2 loads the image 2.gif into the image cache - which allows test
#1 to succeed....

this means that step #1 and step #4 failed because test #2 had not been run
yet... and the image 2.gif was not in the image cache.

Comment 37

[Prashant Desale]

Got it. Verified.