Closed
Bug 137819
Opened 22 years ago
Closed 22 years ago
Changed images don't reload [file:// URLs]
Categories
(Core :: Graphics: ImageLib, defect, P2)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla1.3alpha
People
(Reporter: tbrinkm, Assigned: Biesinger)
References
Details
Attachments
(2 files, 2 obsolete files)
13.93 KB,
patch
|
tor
:
review+
Biesinger
:
superreview+
|
Details | Diff | Splinter Review |
10.75 KB,
patch
|
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
When a page is visited and an image linked to that page is changed, the image is not refreshed when the reload button is pressed. To see the altered image, you must clear the browser's cache. This happens even when 'Compare the page in the cache to the page on the network' is set to 'Every time I view the page'. To verify this: 1 Throw together a page containing an image. 2 View the page. 3 Edit/save the page. 4 Refresh. (Note: The current page is displayed.) 5 Edit/save the image. 6 Refresh. (Note: The old image is displayed.) 7 Clear your cache. 8 Refresh. (Note: the new image is displayed.)
Comment 1•22 years ago
|
||
-> Cache Have you tried shift+reload ?
Assignee: Matti → gordon
Component: Browser-General → Networking: Cache
QA Contact: imajes-qa → tever
Reporter | ||
Comment 2•22 years ago
|
||
Ok. Shift+Reload displays the new image. If you don't mind my asking, why is this required?
Shift+reload reloads from the net. Reload alone reloads from cache. I really wish it was the other way around: I never need to reload from cache. But this is how it's designed to work. Resolving as invalid.
Status: UNCONFIRMED → RESOLVED
Closed: 22 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 4•22 years ago
|
||
I'd accept that if it behaved the same way for the page as it does for the image. Shouldn't images, etc. be handled the same way the page is (i.e.: check to see if the image has changed under the same circumstances the page is checked for the same?) Short of doing that, it's inconsistant behavior at best, a fairly major cache bug at worst.
Comment 6•22 years ago
|
||
I'm going to throw in my two bits on this... I think this is a critical bug, it's going to break some sites for moz. I use a technique that is common, a php script to dynamicly choose an image: img src='/path/to/image.php' It returns a number of headers pleading various browser versions/http versions/whatever to not cache the image returned, sets the Content-Type to image/gif, and returns the image data. Mozilla 1.0 RC1 caches the first image. If I access '/path/to/image.php' directly , and shift-reload, the image changes, but if I then simply hit reload, the image reverts to the first image again (no I am not resending the first image). If I access a page that has an img tag with the src path to the php script, shift-reload will not even reload a new image. No amount of emptying cache, changing cache prefs, etc. changes the behaviour. The script works fine with all non-moz browsers. An example script is below. Shane ------ image.php ------- // place a couple gifs in the same directory as this test file. // start.gif will only be shown the first time the script is accessed // mozilla 1.0 RC1 will always show start.gif, unless shift-reload // do shift-reload then another reload, and moz returns to start.gif if (array_key_exists('done',$_COOKIE)) { $image = 'done.gif'; } else { setcookie('done','1'); $image = 'start.gif'; } $basepath = dirname($_SERVER["PATH_TRANSLATED"]); header("Expires: Mon, 26 Jul 1997 05:00:00 GMT"); // Date in the past header("Last-Modified: " . gmdate("D, d M Y H:i:s") . " GMT"); // always modified header("Cache-Control: no-store, no-cache, must-revalidate"); // HTTP/1.1 header("Cache-Control: post-check=0, pre-check=0", false); header("Pragma: no-cache"); // HTTP/1.0 header('Content-Type: image/gif'); readfile($basepath.'/'.$image);
Comment 7•22 years ago
|
||
This is a problem on Linux too (up to 1.0rc1 2002041711 that I'm using now) and has been for as long as I remember. I would officially confirm the bug if I had permission :)
I would like to confirm that this bug also applies to a file: URL. Just generated a series of analog statistics and loaded them one after another while the images were always the same. I forgot to shift-reload, but cleared the cache instead which made new images appear... I'd like to see page and images in sync, regardless which way this behaviour will be defined, but having them work differently breaks page consistency. This probably needs to be extended to cover scripts and similar stuff, too.
*** This bug has been confirmed by popular vote. ***
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 10•22 years ago
|
||
Any chance at all that this will get fixed before Moz 1.0? I agree with Shane's 'critical' assesment of this bug now, may even be a blocker but I don't know the criteria for going that high. I actually ran into a problem with this 'in the wild' over the weekend. Some sites use a security mechanism of dynamically generated images for a code the user has to manually enter into a text box. They do this to make it more difficult for a bot to influence results. I don't remember where I was at the time though, so I can't include a url for testing. Sorry. Also, if it's a bug on Windows AND Linux, I suspect it's a bug on ALL platforms.
Severity: major → critical
OS: Windows 2000 → All
Hardware: PC → All
Comment 11•22 years ago
|
||
Changing component to HTTP as it involves HTTP reload policy.
Component: Networking: Cache → Networking: HTTP
Comment 13•22 years ago
|
||
reporter, anyone: please provide a testcase. also, this bug seems really old. i suspect this has been fixed since we've not heard of any other reports of this kind of thing. such a major regression would have generated a lot of noise. WORKSFORME please reopen if you can repro this bug.
Status: NEW → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 14•22 years ago
|
||
Not fixed. A static test-case for this can't be made because it involves handling of dynamic content. The process for reproducing this bug is outlined in the description. If you have a dynamic page which uses the same url string to put an image on the page the image will be reloaded from the browser's cache even if the image on the server has changed since it was originally loaded. As an example, I'll go through the process once more. 1. Create an image (untitled.gif). 2. Create a page (test.html) containing the following: - - - - - <html> <head> <title>Image Reload Testcase</title> </head> <body> Sample Text Before Image Change.<br /> <img src="./untitled.gif"/> </body> <html> - - - - - 3. View the page. 4. Change the page contents to: - - - - - <html> <head> <title>Image Reload Testcase</title> </head> <body> Sample Text AFTER Image Change.<br /> <img src="./untitled.gif"/> </body> <html> - - - - - 5. Edit the image in some easily visible way. (change its color or something) 6. Refresh the page, and notice that the text in the page has changed, but you're still seeing the old image. Both the html page and the image have changed on disk since the last time the page was loaded, but only the html is refreshed, the image is still pulled from the browser's cache. Internet Explorer handles this case properly. The image has changed since the last time the page was loaded, so it loads the new image.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 15•22 years ago
|
||
WORKSFORME linux build 2002100808. i used apache 1.3.23 as the server. i created html file and image as described. i loaded the page once. modified the html and the image. and then reloaded the page (using the reload button). it worked as expected. reporter: how are you reloading the page? are you perhaps using file:// URLs?
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 16•22 years ago
|
||
Yes I am currently using a 'file://' url, but when I first discovered this bug I was using a dynamic image generated as part of a PHP script. Why should using a 'file://' url matter? If it reloads the page, it should reload the image as well.
Comment 17•22 years ago
|
||
it matters because the validation process is different for http:// and file://. in http:// we might send a conditional request to the server (such as If-Modified-Since), whereas for file:// there is no equivalent request. my fear is that imagelib might be caching file:// URLs for a fixed amount of time. it might not be properly checking the file modification time. reopening for investigation.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 18•22 years ago
|
||
reporter: does shift-reload help? also, when you used http:// URLs, were you connecting via a HTTP proxy? if so, what is the proxy type/version? thx!
Comment 19•22 years ago
|
||
ok, i'm able to repro this bug when using file:// URLs to load the page. -> imagelib component downgrading severity. i'll investigate this for mozilla 1.3.
Severity: critical → normal
Status: REOPENED → ASSIGNED
Component: Networking: HTTP → ImageLib
Summary: Changed images don't reload → Changed images don't reload [file:// URLs]
Target Milestone: --- → mozilla1.3alpha
Comment 20•22 years ago
|
||
Wow! If it happens even with file:// urls, Imagelib must be incorrectly pulling it out of its cache session. How can we unify some of the caching policies between various cache clients.
Comment 21•22 years ago
|
||
gordon: iirc, imagelib makes up an expiration time for file:// URLs. like maybe 10seconds, or 10minutes. it really needs to stat the files to determine if they have changed.
Comment 22•22 years ago
|
||
Okay, but how about only once per pageload. It could be painful if you have a small image repeated many times on a page.
Comment 23•22 years ago
|
||
true, and i would only stat in the reload case.
Comment 24•22 years ago
|
||
*** Bug 175257 has been marked as a duplicate of this bug. ***
Comment 25•22 years ago
|
||
*** Bug 152259 has been marked as a duplicate of this bug. ***
Comment 26•22 years ago
|
||
Darin, it looks like bug 155275 is a dup of this one. Do you concur? Bug 167514 also seems somewhat similar, but it isn't file: specific, so I'm inclined to leave it as a separate bug.
Comment 27•22 years ago
|
||
*** Bug 155275 has been marked as a duplicate of this bug. ***
Comment 28•22 years ago
|
||
yes, bug 167514 seems different. for that one we should check the load flags for the image load.
Updated•22 years ago
|
Priority: -- → P3
Comment 29•22 years ago
|
||
*** Bug 140670 has been marked as a duplicate of this bug. ***
Comment 30•22 years ago
|
||
*** Bug 180982 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Target Milestone: mozilla1.3alpha → mozilla1.3beta
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: mozilla1.3beta → mozilla1.3alpha
Assignee | ||
Comment 33•22 years ago
|
||
ok now this works. however, I do not know if this is the right way to fix this bug. darin, stuart, please tell me. I will attach a diff -uw version of the patch in a second.
Assignee | ||
Comment 34•22 years ago
|
||
Assignee | ||
Comment 35•22 years ago
|
||
Comment on attachment 107876 [details] [diff] [review] patch Here's my hoping that stuart does read request mails...
Attachment #107876 -
Flags: superreview?(darin)
Attachment #107876 -
Flags: review?(pavlov)
Comment 36•22 years ago
|
||
Comment on attachment 107876 [details] [diff] [review] patch biesi: the patch looks basically ok, but can you please attach a diff with whitespace changes removed? also, how about taking this opportunity to rename httpValidateChecker to something much more appropriate, like imgCacheValidator (or anything else not specific to http).
Assignee | ||
Comment 37•22 years ago
|
||
darin: http://bugzilla.mozilla.org/attachment.cgi?id=107877&action=view is the -uw version of the patch sure, I can rename httpValidateChecker to imgCacheValidator, fine by me.
Assignee | ||
Comment 38•22 years ago
|
||
*** Bug 123156 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 39•22 years ago
|
||
Comment on attachment 107876 [details] [diff] [review] patch clearing requests because I'll attach a new patch in a second
Attachment #107876 -
Flags: superreview?(darin)
Attachment #107876 -
Flags: review?(pavlov)
Assignee | ||
Comment 40•22 years ago
|
||
as above, but s/httpValidateChecker/imgCacheValidator/g in the libpr0n src dir
Assignee | ||
Updated•22 years ago
|
Attachment #107876 -
Attachment is obsolete: true
Attachment #107877 -
Attachment is obsolete: true
Assignee | ||
Comment 41•22 years ago
|
||
Assignee | ||
Comment 42•22 years ago
|
||
Comment on attachment 107936 [details] [diff] [review] patch v2 see the next attachment for the patch without whitespace changes.
Attachment #107936 -
Flags: superreview?(darin)
Attachment #107936 -
Flags: review?(pavlov)
Comment 43•22 years ago
|
||
Comment on attachment 107937 [details] [diff] [review] patch v2 -uw >Index: imgCache.cpp >+ if (SecondsFromPRTime((PRTime)fileLastMod) > lastModTime) >+ *aHasExpired = PR_TRUE; >+ else >+ *aHasExpired = PR_FALSE; no need for an extra branch here, just do this: *aHasExpired = SecondsFromPRTime((PRTime)fileLastMod) > lastModTime; seems fine otherwise, sr=darin (assuming you've thoroughly tested this... e.g., make sure it doesn't negatively impact chrome).
Attachment #107937 -
Flags: superreview+
Assignee | ||
Comment 44•22 years ago
|
||
Comment on attachment 107936 [details] [diff] [review] patch v2 marking darin's sr on the non-uw patch
Attachment #107936 -
Flags: superreview?(darin) → superreview+
Assignee | ||
Comment 45•22 years ago
|
||
Comment on attachment 107936 [details] [diff] [review] patch v2 ok, giving up on waiting for review from stuart tor: So, could you review this patch? It does several things: firstly, if an entry is put into the cache, and the scheme is file:, it sets the metadata attribute to revalidate when the entry is expired. later, in the check if an entry has expired, it checks the last modified time to find out if the entry has expired. thirdly, if the request must be validated, and if a protocol does not impl. nsICachingChannel, the entry is reloaded, instead of the cached version being used. finally, I made httpValidateChecker cope with non-caching channels. ah yeah, and I renamed it to imgCacheValidator. I still have to test if this affects chrome performance, but I don't think it should, because this is mostly file:-specific.
Attachment #107936 -
Flags: review?(pavlov) → review?(tor)
Comment 46•22 years ago
|
||
Comment on attachment 107936 [details] [diff] [review] patch v2 sr=tor Remember darin's change. Tinderbox appears bored at the moment - liven it up with a Ts/Txul test. :)
Attachment #107936 -
Flags: review?(tor) → review+
Assignee | ||
Comment 47•22 years ago
|
||
ok, checked in (with darin's change)
Status: ASSIGNED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 48•22 years ago
|
||
comet's Txul and Ts times: before my checkin: Txul:467ms Ts:1464ms after my checkin: Txul:465ms Ts:1462ms -> no measurable impact.
Comment 49•22 years ago
|
||
*** Bug 189019 has been marked as a duplicate of this bug. ***
Comment 50•22 years ago
|
||
*** Bug 191520 has been marked as a duplicate of this bug. ***
Comment 51•22 years ago
|
||
Looks fixed in OS/2 trunk 2003021213.
Comment 52•21 years ago
|
||
*** Bug 136109 has been marked as a duplicate of this bug. ***
Comment 53•21 years ago
|
||
*** Bug 211726 has been marked as a duplicate of this bug. ***
You need to log in
before you can comment on or make changes to this bug.
Description
•