79063 - Reloading frameset ignores changes to src attribute of <frame>

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Xuân Baldauf]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9+) Gecko/20010503
BuildID:    2001050304

You have a document which contains a frameset definition. After
reload, the frameset definition changes, so the referenced frames have to be
reloaded. Netscape Communicator 4.77 does that, but mozilla does not.

Reproducible: Always
Steps to Reproduce:
1. Go to http://justchat4.medium.net:4000/test/mozilla/framesTest/
2. You'll see two frames where each frames contains a random string.
3. Hit the "Reload" button
4. Check for changes of the random string
5. Hit the "Reload" button
6. Check for changes of the random string
7. Click into the URL text field and hit the "return" key.
8. Check for changes of the random string

Actual Results:  At step 4 and 6, you do not see any change. At step 8, you see
changes.

Expected Results:  At steps 4, 6 and 8, you should see changes. You should not
see the same strings again.

The server sends a "Cache-Control: no-cache" and "Pragma: no-cache" HTTP
response header to force proxies and clients to not cache the result. Mozilla
does seem to obey, but only when the "return" key is hit at the URL text field.
Mozilla does not obey when "Reload" or Shift+"Reload" was hit.

One interesting aspect: once you got a fresh response from the server displayed,
you can hit "reload" again. You will not see the fresh response, but the cached
response from the first request to the server.

I suspect an URL comparision bug, because the old and new URls for a given frame
only differ in the query part.

Comment 1

[gordon]

Darin, I'm not sure if this is HTTP or something higher up.

Comment 2

[David Einstein]

I'm digging.

Comment 3

[Darin Fisher]

Attachment: http log for this testcase

Comment 4

[Darin Fisher]

from the http log it is apparent that necko is fetching the toplevel page and
the frame in each case (reload, shift-reload, and enter).

over to layout for further investigation.

Comment 5

[David Einstein]

Yes,
  Somehow if we are reloading the page due a call from History after we send
nsHttp* off to reload the page, but ignore the result.  

If you go forward and back from this page it also will not display an updated page.

If you select the page from the sidebar history, or hit the go button or hit
return in the URLbar it will load.
I'm pretty sure that this should go -> Session History but I'll let it wander.

Comment 6

[David Einstein]

My bad. 
Doing a view source on the window shows the correct new frame, it's just that 
the frameset is loading the wrong frames.
It is layout.  I'll figure out how this thing works eventually.

Comment 7

[David Einstein]

Apologies for the spam.
This is a Session History bug. We've got the correct URI going into
nsDocShell::LoadURI, but then find the wrong one in the history.
He's right! Everything I do know is wrong!

Comment 8

[Tom Mraz]

Certainly not platform specific bug.
I've seen that bug on many pages with changing content in frames. When I want to
reload some of the frames it is impossible.
Raising the severity since it is almost impossible to use Mozilla on framed
pages with changing content.

Comment 9

[karnaze (gone)]

Reassigning to pollmann.

Comment 10

[Chris Petersen]

Frames related. Changing QA to Amar

Comment 11

[Eric Pollmann]

Handing Session History issue to Radha.

Comment 12

[Eric Pollmann]

Really reassigning.  :)

Comment 13

[Jesse Ruderman]

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

Comment 14

[Jesse Ruderman]

The reporter of bug 89584 (Graham) included a testcase that shows that Mozilla
does reload the frame, but reloads the wrong URL into the frame.

http://www.computer-programmer.org:8080/jalindi/servlet/moztest.mozbug

Comment 15

[Bradley Baetz (:bbaetz)]

This is a problem for iframes as well

Comment 16

[Alfonso Martinez]

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

Comment 17

[Radha on family leave (not reading bugmail)]

I expect the fix for 93027 which went into trunk today to fix this. Holding on
until I can verify this in latest build.

Comment 18

[Radha on family leave (not reading bugmail)]

Looking deeper, I think this is not related to 93027. Working ...

Comment 19

[Radha on family leave (not reading bugmail)]

Darin,
I need some help from you on this. As described earlier by David Eistein on
6/20/01, the new updated uri comes for loading in nsDocShell::LoadURI(). But
since  reload button was pressed, docshell, takes the uri for the subframes from
session History (which happens to be the old random string in this case) and
loads it. 

I can fix it in docshell, if I can get information from cache that the page was
loaded with "no-cache" and thereby any subsequent loads will be fetched from the
server. Session History already sniffs for cacheKey and cacheToken. In this
example, the top level url as well as the 2 subframes have cacheKeys and
cacheTokens. So, session history thinks that it is a regular load and proceeds.
Is there someother info I can get from nsICachingChannel to determine the
"no-cache" status?

Comment 20

[Bradley Baetz (:bbaetz)]

Note that shift-reload doesn't fix this test case.

Why does SH know about subframes? We must reparse the document each time -
anything else will just lead to broken behaviour, won't it?

Comment 21

[Radha on family leave (not reading bugmail)]

No. SH needs to know all about framesets.

Comment 22

[Bradley Baetz (:bbaetz)]

Why? for back and forward?

In any event, if it does, it needs to know to invalidate what it does know when
the underlying document changes.

Comment 23

[Darin Fisher]

radha: you can check the expiration time of the cache entry to determine whether
or not it was given as "no-cache" content:

given a cache token, QI the cache token to a nsICacheEntryInfo (see
nsICacheVisitor.idl).  then call GetExpirationTime and check if the
expiration time is in the past.

note: this will not work when "no-cache" is given in a HTML <meta> tag.

Comment 24

[Radha on family leave (not reading bugmail)]

Darin,

Are the steps you have described the shortest way to do this. Is there a
convinience routine anywhere off of nsICachingChannel that will accept a
cacheToken and provide a boolean status about its expiration. If not, Is it
possible to add one? The reason I ask is, I want to minimize the interaction of
docshell with the workings of cache, so that docshell won't be impacted if the
mechanism underneath changes.   

Also, is it safe to assume that if a page haa 'no-cache', if also implies a
no-store Thanks,

Comment 25

[Darin Fisher]

radha:

1) no such convenience routine exists... feel free to add one to nsNetUtil.h

2) no-cache means a 0 freshness time (or expiration time == now)
   no-store means that the data is not stored in the cache

Comment 26

[Radha on family leave (not reading bugmail)]

Darin,

Regarding the no-cache and no-store thing, I understand the difference. If a
page was loaded with, no-cache, will all future loads of that page( either thro'
history mechanisms like back/forward or otherwise) be fetched from the server,
so that any form values  etc... willbe reset to the original value. 

Comment 27

[Darin Fisher]

if a page was loaded with 'no-cache' then the Back/Forward behavior will be to
use the cached copy without contacting the origin server.  i would think that
this means that we should not reset the form contents.  'no-store' content must
however be fetched since it is not kept locally, and thus i would expect that
the form values be reset as well.

Comment 28

[Graham Robertson]

From what I've read 'no-store' means that the data should not be stored because
there is potentially sensitive information on the page such as a credit card 
number or personal information.
The 'no-cache' means that the page expires immediately and therefore the back 
and forward button should reload the page because the page has expired. This is, 
I think, the behaviour of Explorer and Netscape 4.x.

Comment 29

[Xuân Baldauf]

AFAIK RFC2616.13.13 "History Lists" states

   History mechanisms and caches are different. In particular history
   mechanisms SHOULD NOT try to show a semantically transparent view of
   the current state of a resource. Rather, a history mechanism is meant
   to show exactly what the user saw at the time when the resource was
   retrieved.
So pages tagged as "no-cache" should not be cached (that is, requests to the
same URL need to be forwarded to the origin server, not served locally by
cache). But using "back" and "forward" should not generate a request, even if
the data to be shown is expired.

The current implementation in Explorer and Netscape 4.x is evil. Imagine that
you remember that you wanted to write down something you wrote into a form
submission page a minute ago. Those browsers won't let you see what you submitted.

RFC2616.14.9.2 also states about "no-store" tagged pages "History buffers MAY
store such responses as part of their normal operation."

So it is fine to show the pages exactly as they were originally received.


P.S.: For history in general, it would be nice to have a 2-dimensional history,
which is formed as a tree: Every current view is a node, and every change of a
view (due to clicking in links, etc.) creates a descending node of the current
node. There may be more than one descending node of a given node, because people
can return to a previous view (using the "back" button) and then click on other
links. But what if they want to return to the original descending node? This is
not possible with current browsers, but maybe with mozilla of the future :-)

Comment 30

[Darin Fisher]

Xuan:

i couldn't agree with you more; however, we found that web sites (especially
banks) needed a way to instruct mozilla to "behave like 4x" and so we decided to
use "no-store" to mean don't put it in the cache, even though the spec says that
we can (for the purposes of session history).

Comment 31

[Xuân Baldauf]

I think, for history, there is a special kind of cache (access pattern) needed. "

"no-cache" pages should be cacheable both on disk and in RAM, at least for the
session lifetime (session as understood as an open browser window). Only
"history-requests" to those cached pages should be able to succeed.

"no-store" pages should be cacheable only in RAM, at maximum for the session
lifetime. Only "history-requests" to those cached pages should succeed. There
should be a user-configurable timeout setting when those pages should be
forgotten from RAM (e.g. "after 5 minutes", "always", "up to session lifetime",
"only 5 history entries away" (which means that if you have clicked 5 times onto
linked pages, history forgets about such a page)). The default for such a
timeout should be "always" (or "0 minutes" or "0 history entries away"), so
banks can be happy with that. Some users (like me) will want to override that
what web sites tell their browser.
Is that an acceptable solution for both power users and novices?

Comment 32

[Darin Fisher]

i like your suggestion, and we've actually considered that, but unfortunatly
banks don't want it to be a user preference, because otherwise they can't
guarantee a certain caching/history behavior.

Comment 33

[Xuân Baldauf]

Mhh... Banks should not dictate what is a decision on the client (especially if
the client is open source). Maybe we can tweak a little bit here: 

Option (1): The "forget no-store" timer configuration can be within "prefs.js"
and the like. If people do not find it, they won't change it.

Option (2): Let banks decide on high-security https sessions, but if they use
http-sessions, they already loose (from security point of view), so letting the
user decide in this case cannot be a very significant loss of security.

Comment 34

[Darin Fisher]

the bank in question did not like the fact that SSL content could be revisited
with the Back button.  so, perhaps we need only need to restrict 'no-store' SSL
content.

Comment 35

[Xuân Baldauf]

I can live with that if only SSL content is restricted and decisions about
"unsecure content" are configurable. (They may be restricted as a default).

Comment 36

[Darin Fisher]

if i'm not mistaken we've drifted from the original bug report... perhaps our
dicussion (or resolution) should be copied into a new bug report.  Xuan: if i've
understood correctly, can you do that?  thanks!

Comment 37

[Radha on family leave (not reading bugmail)]

Attachment: patch to docshell

Comment 38

[Darin Fisher]

radha: the patch looks fine to me, but as i mentioned in some other bug, you
could replace PRTimeToSeconds(PR_Now()) with PR_IntervalToSeconds(
PR_IntervalNow()), which would eliminate PRTimeToSeconds from your patch.

Comment 39

[rpotts (gone)]

hey radha,

i'll defer to darin on the PRTime issue - i have a very tenuous grip on time
right now :-)

the patch looks good to me... (r=rpotts)

my one comment is that i don't think that you need to prototype
'ShouldAddToGlobalHistory(...)' as taking a PRPackedBool.  A PRBool should be
fine.  The compiler should always be able to promote a PRPackedBool to a PRBool
(if necessary).  So, its fine to declare variables as PRPackedBool and pass them
into methods that take PRBool arguments.
I think that PRPackedBool should only be used as a storage type.  everywhere
else PRBool should be used...
-- rick

Comment 40

[Darin Fisher]

i agree with rick on the issue of PRPackedBool vs PRBool... PRPackedBool is (i
think) intended to be used for class member variables.  a compiler may choose to
save some space by packing say 4 PRPackedBool's into one 32-bit integer, in
cases in which byte-aligned access is not inefficient.  but, for stack variables
in which you are likely just declaring a single boolean variable, using
PRPackedBool probably won't make any difference.

Comment 41

[Radha on family leave (not reading bugmail)]

Darin,
I tried to use your suggestion regarding PR_IntervalNow(). But that doesn't work
out right. The expirationTime returned by nsICacheEntryDescriptor() is greater
than the value returned by PR_IntervalToSeconds(PR_IntervalNow()). Here's my
code snippet and the results I get.

cacheEntryDesc->GetExpirationTime(&expTime);         
PRUint32 now = PR_IntervalToSeconds(PR_IntervalNow());         
printf("Exptime = %d, Now = %d\n", expTime, now);
if (expTime <=  now)            
   expired = PR_TRUE; 

Results:
Exptime = 999716814, Now = 73306   

Maybe I'm using it wrong or my logic is wrong? Please advise. Thanks,

Comment 42

[Darin Fisher]

radha: do you get different results if you use PRTimeToSeconds()?

it's not invalid for the expiration time to be sometime way in the future, but
this does seem a bit excessive.  however...

i bet the problem is that PR_IntervalNow is taken from system startup, whereas
PR_Now is taken from 00:00:00 Jan 1, 1970.

perhaps you should just use PRTimeToSeconds for now until we get it added to NSPR.

Comment 43

[Radha on family leave (not reading bugmail)]

Attachment: updated patch

Comment 44

[Radha on family leave (not reading bugmail)]

darin: I have posted an updated patch which incorporates rpott's suggestion.
This patch still uses PR_Now(). Can you put a sr comment, so that I can try get
a approval from mozilla.org. Thanks,

Comment 45

[Darin Fisher]

sr=darin

Comment 46

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Bug 98887 (printing to console in optimized builds) should be fixed before this
lands on a branch.

Comment 47

[Radha on family leave (not reading bugmail)]

This has been fixed in the trunk, but did not make it to the 0.9.4 branch. 

Comment 48

[Tom Mraz]

Verified Linux 2001091121

Comment 49

[Amarendra Hanumanula]

 Changing QA contact to Claudius

Comment 50

[Graham Robertson]

I'm not sure this bug is completely fixed.

If I put a button with the following
<button onclick="window.top.document.location.reload(true); 
alert('hello')">reload</button>

in it, I get the same old problem. Therefore, I my application still
does not work properly

Comment 51

[Ulrich Grassberger]

Well my test code works, 0.9.6 Win32. 
My code is not using 

<button onclick="window.top.document.location.reload(true);
alert('hello')">reload</button>

but jumps in page history via javascript, history.back(), in
order to change a certain frameset if another one is changed.
Mozilla 0.9.2 didn´t record these jumps on the history list,
0.9.6 does. E.g. http://grassomusic.de/english/acpi.htm

Comment 52

[Ulrich Grassberger]

>but jumps in page history via javascript, history.back(),
>in order to change a certain frameset if another one is 
>changed.
I meant "frame", not "frameset". 

But as Graham said if one reloads the whole frameset via javascript Mozilla 
loads the initial pages of the frame. This is a bug also because it is 
inconsistent behaviour: If one reloads the frameset via the "reload" button 
Mozilla doesn´t load the initial pages but the actual ones.  

A related bug: If you bookmark a frameset, IE recollects the actual frames, Ns4 
and Mozilla 0.9.6 don´t. 

I think all this deals about a fundamental problem of frames: What do the 
navigation buttons, the adress line and the whole document concept refer to - to 
the bare frameset page with its initalization code _OR_ to the actual pages? 

When Ns invented frames, they should have built the whole concept with a 
variables handoff like frameset.htm?frame1URL&frame2URL. I have been worriing 
about this and imagined if javascript could write the content of the URL input 
box, i.e. "fake" it, and modify page history - I would write my own frameset 
implementation. Any thoughts?

Comment 53

[Radha on family leave (not reading bugmail)]

Reporter, Do you have a url that has the button to reload the page through
javascript?  BTW, the original bug itself has regressed in themean time due to
some changes in the cache. I have a solution to take care of the original problem. 

Comment 54

[Xuân Baldauf]

Sorry, I do not have a test case where the reload is done via JavaScript. But,
with Mozilla 2002042909, the bug seems the same as one year ago. The initial bug
description still applies.

Comment 55

[Ulrich Grassberger]

http://grassomusic.de/temporary/frameset.htm

Comment 56

[Radha on family leave (not reading bugmail)]

The original problem and the testcase attached for reload from JS work now. A
small one-liner fix that went in as part of fix for bug 125372 (yesterday) fixed
this problem.