Closed
Bug 1083072
Opened 10 years ago
Closed 10 years ago
Logo image shows up randomly after refreshing the webpage
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
People
(Reporter: jmdelprato, Assigned: seth)
References
()
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
application/x-zip
|
Details |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.13 Safari/537.36
Steps to reproduce:
I'm currently working on a WordPress website (WP 3.9.2) and I'm loading my main logo with a simple <img /> tag as you can see on this page :
http://showbuzz.org/tell-me-more/?page_id=84
On the Firefox 32 branch, this image shows up randomly on each refresh. I don't encounter this issue on Firefox 31 and below. As soon as I open up the inspector, the image appears immediately which makes it difficult to debug.
Actual results:
- The image is randomly loaded, the page actually seems to stop rendering before it loads the image.
Expected results:
- The logo should appear on every refresh (as it does on Firefox 31 and below or other browsers).
I'm able able to reproduce it with FF33 and FF35. I think it's an issue about cache too (maybe cache v2).
Component: Untriaged → Networking: Cache
Product: Firefox → Core
Summary: With Firefox 32.0, my logo image shows up randomly. Some refreshes fully load it, some don't (it seems like a cache issue) → Logo image shows up randomly after refreshing the webpage
Keywords: regressionwindow-wanted
Just a quick comment :
Since the issue only shows up with the main logo, I looked at its particularities --- it's a WordPress install, and the logo isn't hardcoded : it is loaded with a PHP call.
<div class="site-logo">
<a href="http://www.showbuzz.org/tell-me-more/index.php?accueil=1" title="<?php echo esc_attr( get_bloginfo( 'name', 'display' ) ); ?>" rel="home">
<img data-altlogo="<?php echo $alternate_logo; ?>" src="<?php echo $sitelogo; ?>" alt="<?php echo esc_attr( get_bloginfo( 'name', 'display' ) ); ?>">
</a>
</div>
As you can see, there's a "data-altlogo" attribute before the actual "src" within the <img> tag. For the sake of simplicity, I've hardcoded the src attribute and deleted the "data-altlogo" attribute. Upon dozens of refreshes, I haven't yet been able to reproduce the issue yet on FF32, FF33 and FF34 :-)
Can you confirm that you cannot reproduce the bug anymore ?
Would you like me to put the older code live again in order to debug it ?
Best regards
Argh, if you fixed on your side, we can't know if it's a bug in Firefox. :D
My guess is it's a bug, I tried with previous versions and the logo was loaded at each refresh.
Could you cancel your change if possible? I'll like to run mozregression.
Flags: needinfo?(jmdelprato)
Sorry :-)
It's ok, I've put the older code back again -- the logo shows up randomly on FF32, etc.
Flags: needinfo?(jmdelprato)
Regression range:
good=2014-05-31
bad=2014-06-01
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=323156681cef&tochange=d6fccb7c56a8
My guess is it's a side-effect of bug 870021.
Do you consider this bug as "confirmed" yet ? Can I put the fixed-code back again ?
Thanks in advance,
Best regards
Yes, I confirm it. It would be nice to have the testcase "working" until a Moz dev check the issue. I'll CC someone.
Flags: needinfo?(jschoenick)
Hi again ! The website is intented to go to production tomorrow so I might reactivate the correct code on 11am.
If you want to test that bug again, I could duplicate its current state and give you another URL. Don't hesitate !
Best regards
Comment 9•10 years ago
|
||
Bug 870021 regression from: https://hg.mozilla.org/mozilla-central/rev/4a326b265f41
What's happening is:
- nsImageFrame is created, calls OnStartContainer(). At this point the image has imgIRequest::STATUS_SIZE_AVAILABLE, so we set our intrinsic size
- we hit ComputeSize, and ask the imageLoader for the natural size of the image, except the request suddenly does NOT have size available and is 0x0, so we get 0x0.
- we get a Notify(SIZE_AVAILABLE) and call OnStartContainer again, but since the intrinsic sizes didn't change between init and now, we don't invalidate the frame, so we never update the new natural size in ComputeSize
The patch above was assuming image natural size would only change when intrinsic size did, but we seem to go through | size available -> not -> available again | states with the same request (some cache black magic at the network layer?), which means its not safe to rely on size being available in ComputeSize.
@seth - do you know why this might be happening, and if it suggests there's a deeper bug? Does it sound reasonable to just cache natural size in nsImageFrame::UpdateIntrinsicSize?
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to John Schoenick [:johns] from comment #9)
> @seth - do you know why this might be happening, and if it suggests there's
> a deeper bug? Does it sound reasonable to just cache natural size in
> nsImageFrame::UpdateIntrinsicSize?
It sounds like there's a deeper bug somewhere, absolutely. We should try to fix that rather than working around it. Once we have a size available, it should stay available and it shouldn't change, except in the case of multipart/x-mixed-replace images. (And it doesn't seem like that's what we're dealing with here.)
Without investigating more deeply, I'm not sure precisely where things are going wrong. It'd be useful to verify that the current request on nsImageLoadingContent never gets changed and that imgRequestProxy::ChangeOwner never gets called on the imgRequestProxy associated with the request (in other words, on the imgIRequest object).
If those things are true, I'd want to understand why we fail to get the natural size - presumably RasterImage::GetWidth is failing, but I don't understand how it could possibly ever recover in that case, since it only fails if RasterImage::mError is true, and once we set it to true we never unset it.
Flags: needinfo?(seth)
Reporter | ||
Comment 11•10 years ago
|
||
Thank you guys, somehow I woke up this morning feeling useful :)
Comment 12•10 years ago
|
||
[Tracking Requested - why for this release]: recent regression since FF32, but it could be maybe fixed in FF34 or later.
status-firefox33:
--- → affected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Comment 13•10 years ago
|
||
(In reply to Loic from comment #12)
> [Tracking Requested - why for this release]: recent regression since FF32,
> but it could be maybe fixed in FF34 or later.
Before tracking, can you confirm that this issue impacts 34+?
Comment 14•10 years ago
|
||
Yes, in general, I don't report a regression already fixed in advanced versions. ;)
As the reporter said, the website will go in production and he has probably fixed the issue with the workaround in comment #2, but the underlying issue is still present.
Comment 15•10 years ago
|
||
Given that this has already been fixed on the site, I'm not going to track. I have marked 33-36 as affected.
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
tracking-firefox34:
? → ---
tracking-firefox35:
? → ---
tracking-firefox36:
? → ---
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Jean-Marc from comment #11)
> Thank you guys, somehow I woke up this morning feeling useful :)
Thanks for reporting this!
You had offered to duplicate the version of the site that demonstrated the bug so that it would remain accessible; could you please do that if you haven't already? This is definitely something we want to fix, but I'm guessing it may take some time to debug.
Reporter | ||
Comment 17•10 years ago
|
||
Hi, thank you for your kind comment, I have indeed updated the code in order to fix the issue (cf. comment #2). I will duplicate the site on another server (with the original, error-prone code) and I will publish a test-URL by tomorrow.
Updated•10 years ago
|
Flags: needinfo?(jschoenick)
Reporter | ||
Comment 18•10 years ago
|
||
Just a quick note :
The site went finally on production yesterday, on another web server.
I've put the original code back on the development server, accessible through the URL I've submitted in the first post ... and it will stay there as long as you need it !
With FF 32 and now FF 33.0.2, the URL http://showbuzz.org/tell-me-more/?page_id=84 randomly shows the logo on the upper left corner of the page.
Comment 19•10 years ago
|
||
Steps to reproduce:
1. Open attached index.html
2. Reload(F5), repeat step2 if necessary
Actual results:
Image disappears
Expected results:
Image should not disappear.
Page layout should be consistent after reload
Regression window
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8164fe57ac92&tochange=9ae055051998
Suspect: Bug 870021
Comment 20•10 years ago
|
||
[Tracking Requested - why for this release]:
This is regressed since Firefox32
It should be fixed on next ESR38.
status-firefox37:
--- → affected
status-firefox38:
--- → affected
status-firefox39:
--- → affected
tracking-firefox38:
--- → ?
tracking-firefox39:
--- → ?
Updated•10 years ago
|
status-firefox-esr31:
--- → unaffected
Assignee | ||
Comment 21•10 years ago
|
||
We actually tracked this down recently. Bug 1141376 and bug 1019840 together should fix this.
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(john)
Tracking for 38 and 39.
Assignee | ||
Comment 23•10 years ago
|
||
OK, bug 1141376 and bug 1019840 both landed this morning. (And will be in the next Nightly.) So we just need to verify the fix and then uplift.
Assignee: nobody → seth
Assignee | ||
Comment 24•10 years ago
|
||
Jean-Marc, I'm sorry it took so long to get this fixed. This was a tricky one.
The fix should be in the Nightly that comes out tonight. Could you verify that it's fixed now? I can't reproduce the issue here.
Flags: needinfo?(jmdelprato)
Assignee | ||
Comment 25•10 years ago
|
||
We really need verification of the fix here. Can anyone who could reproduce this problem previously confirm that it's fixed?
Keywords: verifyme
Reporter | ||
Comment 26•10 years ago
|
||
Sorry for my late answer and many thanks for taking the time to address this issue...!
Under FF 36.0.4, I'm not able to reproduce the problem any more upon many refreshes (that was actually the problem -- it showed up randomly). Would you like me to test it on other versions as well ?
Thanks
Flags: needinfo?(jmdelprato)
Comment 27•10 years ago
|
||
(In reply to Jean-Marc from comment #26)
> Sorry for my late answer and many thanks for taking the time to address this
> issue...!
> Under FF 36.0.4, I'm not able to reproduce the problem any more upon many
> refreshes (that was actually the problem -- it showed up randomly). Would
> you like me to test it on other versions as well ?
> Thanks
The original issue is only fixed in FF39 (Nightly) at the moment, but your testcase has been fixed by... yourself according to your comment #8. :)
You should test the old version of your website, before he went to production, with FF39 if you really want to confirm it's fixed or not.
Comment 28•10 years ago
|
||
(In reply to Seth Fowler [:seth] from comment #25)
> We really need verification of the fix here. Can anyone who could reproduce
> this problem previously confirm that it's fixed?
Working range according to the testcase posted by Alice:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cf1060d8ce9f&tochange=4d2d97b3ba34
Your 2 bugs are present, so it's fixed for me.
Assignee | ||
Comment 29•10 years ago
|
||
Excellent! Thanks for the verification, Loic.
Jean-Marc, if you get a chance to test against the old version of that website, more verification is always better. I know that may be tricky, though.
I'm going to go ahead and mark this as resolved.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Looks like patches on bug 1141376 and bug 1019840 should land soon on aurora. Since this is resolved for 39 I'll untrack it for 39 and leave it tracked for 38. Nice to see this tricky issue resolved!
tracking-firefox39:
+ → ---
Assignee | ||
Comment 32•10 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #31)
> Seth, can we uplift that to 38? Thanks
It's already in 38. There's no patch in this bug; the uplift happened in bug 1141376 and bug 1019840.
Flags: needinfo?(seth)
Assignee | ||
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•