313440 - Missing images or plugins in several popular real estate virtual tours

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Brendan Eich [:brendan]]

<brendan> anyone around able to confirm a regression from firefox 1.0x?
<brendan> http://r2.virtualtourhosting.net/ListingList.php?id=208|detect
<brendan> main left and right panels don't load images
<brendan> i'm on linux, so the panoramic scenes try to load in mplayer, which has its own issues
<brendan> this link works in 1.0x
<brendan> (once you click past the popup-blocker workaround)
<brendan> another popular virtual real estate tour seems busted, let me find an example
<brendan> here: http://tours.tourfactory.com/tours/sizetour.asp?t=235918&sreferer=www.mlslistings.com&home=www.mlsli
<brendan> in 1.0.x, you can click on the 2nd, ... scenes
<brendan> try the last one, the highest tab row, on the left: Back
<brendan> (the back yard)
<brendan> nothing, and the space for the image is vertically collapsed
<brendan> same for all scenes but the first
<brendan> works in 1.0.x
<brendan> not in my trunk build, or in my deer park b2
<gavin> those seem to work fine for me using windows/latest-branch
<gavin> can't help with linux testing, sorry
<gavin> though switching from "html viewer" to "java viewer" and back did cause a hang :(
<brendan> gavin: is this with 1.8branch nightly
<gavin> yes
<gavin> Gecko/20051021 Firefox/1.5
<brendan> try trunk if you can
<brendan> this could be a trunk-only
<brendan> regression
<gavin> with the embed/object changes I'd say that's much more likely
<gavin> ok, first link doesn't load images, using 20051020 trunk
<gavin> second link seems to work fine
<gavin> I'll find a regression range
<brendan> thanks!
<gavin> brendan: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-10-16+05%3A00&maxdate=2005-10-17+08%3A00&cvsroot=%2Fcvsroot
<gavin> seems like bug 114641 or bug 288064 are likely

/be

Comment 1

[Hermann Schwab]

The hang on the second URL is seen as crash in
Bug 312495 crash when exiting a page with Java Bubble viewer

Comment 2

[Frank Wein [:mcsmurf]]

Attachment: Testcase

Requires Flash plugin

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

The problem here is apparently that ContentStateChanged causes frame reconstruction async; OnStartRequest treated lack of a frame as a problem and fell back. this patch changes that.

Unfortunately it also makes us request the data twice. I think we should change things so that we construct frames sync here.

Also, I only tested with the attached test case. Could someone test on the URLs reported in comment 0?

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: complete patch, including sync frame construction

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 200535 [details] [diff] [review]
complete patch, including sync frame construction

>Index: content/base/src/nsObjectLoadingContent.cpp
>+    {
>     mozAutoDocUpdate(doc, UPDATE_CONTENT_STATE, PR_TRUE);
>     doc->ContentStatesChanged(thisContent, nsnull, changedBits);
>+    }

Fix the indent here, please, and document that we want the update to be done before we do the FlushPendingNotifications call, ok?

>Index: content/base/src/nsObjectLoadingContent.h
>+    void NotifyStateChanged(ObjectType aOldType, PRInt32 aOldState,
>+                            PRBool aSync = PR_FALSE);

There are only two callers of this method.  Just make the arg non-optional, and change both, please.

r+sr=bzbarsky with that.

Could you file a followup bug (cc me, dbaron, roc) about RecreateFramesForContent not flushing?

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch for checkin

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in. someone please verify that the URLs in comment 0 are fixed.
Bug 313516 filed for RecreateFramesFor flushing

Checking in content/base/src/nsObjectLoadingContent.cpp;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.cpp,v  <--  nsObjectLoadingContent.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in content/base/src/nsObjectLoadingContent.h;
/cvsroot/mozilla/content/base/src/nsObjectLoadingContent.h,v  <--  nsObjectLoadingContent.h
new revision: 1.2; previous revision: 1.1
done