Closed Bug 86075 Opened 24 years ago Closed 24 years ago

<object> causes table width to become enormous

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: jiang_wq, Assigned: piskozub)

References

()

Details

(Keywords: regression, testcase, top100, Whiteboard: critical for 0.9.2, r=peterl, sr=jst,hyatt,a=blizzard for patch 39636)

Attachments

(5 files)

From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.1+) Gecko/20010614 BuildID: 20010614 go to http://astrology.yahoo.com and click "romance", there should be a drop down list for you to pick a sun sign. Mozilla fails to display that list. Reproducible: Always Steps to Reproduce: 1.go to http://astrology.yahoo.com 2.click on "romance" 3.there's no where to pick a choice in the new page
Works For Me in build 2001061404 in WinNT. (same drop-down lists as IE 5.5)
On w2k build 2001061404 it works sporadicaly, sometimes the whole page is displayed, sometimes just part. The ad at the bottom of the page is always clipped in moz though. I am running with all caching disabled and repeated reloads will sometimes bring up the whole form, and sometimes just a part. If I can trust view source to accurately represent the source of the pages there is no significant difference between the sources of the pages that display differently. this should probably go to layout.
Attached image A snapshot of the badly displayed page (deleted) —
It's not totally obvious in David Einstein's screenshot but what seems to be happening is that the page becomes _enormous_ width ways. All the elements may actually be present, but something makes the page become huge and they presumably vanish off 8000 pixels right or something. I'm sure I've seen symptoms like this before but I can't remember where :( Reproduced Linux 2001061421
What ruth@innocent.com said. When I load the romance page, a bunch of drop-down controls appear about a mile and a half off to the right. -> Layout. 2001061506/Linux
Assignee: asa → karnaze
Status: UNCONFIRMED → NEW
Component: Browser-General → Layout
Ever confirmed: true
OS: Windows 2000 → All
QA Contact: doronr → petersen
Attached file boiled bug, yum! (deleted) —
OK. Typical of me to see romance as broken when it was only larger than it was supposed to be :-/ The problem seems to be something in the <OBJECT> tag. Note: I do not have the flash plugin.
nice testcase. updating summary.
Summary: form not show up in mozilla → <object> causes table width to become enormous
Looks like a possible regression of bug 5522
*** Bug 86362 has been marked as a duplicate of this bug. ***
as the DUP notes, this thing is affecting other major sites like http://www.foxnews.com/ and http://www.nhl.com/ Oddly enough, if you change the font size (or even select the same font size off of the menu) the page renders properly. It also renders properly in Composer.
*** Bug 86340 has been marked as a duplicate of this bug. ***
*** Bug 86272 has been marked as a duplicate of this bug. ***
Another fine example of this bug: http://www.firaxis.com/civ3/ For me at this URL and on all foxnews pages this happens 100% of time (recent Win32 builds on WindowsME) Mozilla0.9.1 was free of this bug (see attachements to duplicate bug 86272)
Build 2001-06-12-09-trunk (win98se) seems ok. Build 2001-06-12-22-trunk has this bug.
The problem is likely in the change to nsObjectFrame.cpp version 1.220. In particular the code that is now at line 899 and looks like // finish up if (NS_FAILED(rv)) { // if we got an error, we'll check for alternative content with // CantRenderReplacedElement() nsIPresShell* presShell; aPresContext->GetShell(&presShell); presShell->CantRenderReplacedElement(aPresContext, this); NS_RELEASE(presShell); } else { NotifyContentObjectWrapper(); } The code used to return OK, even in case of failure. Now it returns failure triggering an assertion in debug Mode, and aboring gobs of reflow code. There are a bunch of competing bugs in this area (bug 76085 , bug 64645 for starters) so I'm not going to even guess at a fix.
Changing QA contact
QA Contact: petersen → bsharma
Adding testcase tothe keywords as we have attachement 38660. Changing platform to All.
Keywords: testcase
Hardware: PC → All
*** Bug 86441 has been marked as a duplicate of this bug. ***
Adding regression keyword. This bug seems to ruin all sites using flash. Strangely enough, they also seem to appear several times now, not only once and way off like they used to. Check out the site from bug 72427: http://www.harrypotter.com
Keywords: regression
This bug has a normal severity, no priority, and no nsdogfood keyword. Am I the only person that thinks this is a little odd? Showstopper right here! The average user will want to see a (cool awesome neato) Flash site, but it appears nothing is on the page...drats.
I do. I changed the severity to major ("major loss of function"), added nsbeta1 and nsCatFood (nominated for "a serious user satisfaction" problem). It would be good if someone found a top100 website showing this problem.
Severity: normal → major
Keywords: nsbeta1, nsCatFood
astrology.yahoo.com is part of the top #1 site, yahoo.com, so I think it counts.
Whiteboard: critical for mozilla 0.9.2
Yes, and so does www.mtv.com from dup bug 86441. Anyway, you've beet me to top100 ;-) Seems we made it a Christmass tree bug very fast!
Whiteboard: critical for mozilla 0.9.2
That's the spirit! :) That's much better. One more question; where are you getting your top100 info? This would be invaluable information for me to use on other bugs.
Click the link on the word Keyword up this page. Find top100 and follow the link which BTW is http://komodo.mozilla.org/buster/pagelist.html Sorry for the SPAM but more people may find this useful.
*** Bug 86610 has been marked as a duplicate of this bug. ***
*** Bug 86663 has been marked as a duplicate of this bug. ***
If this can help, here is another page showing the same behaviour: http://www.matrox.com/mga/archive_story/jun2001/g550_feature.cfm Sorry for the spam and the dupe. Shockwave 5.0r41 installed, Win2Ksp2
*** Bug 86779 has been marked as a duplicate of this bug. ***
Keywords: mostfreq
Whiteboard: critical for 0.9.2
*** Bug 86837 has been marked as a duplicate of this bug. ***
Confirming that the problem started between the morning builds of 06/12 and 06/13. It would fit the 1.220 change to ../layout/html/base/src/nsObjectFrame.cpp David wrote about on 06/18. It was checked in 6/12 19:42. The reason was "bug 76085 - remove the effects of the patch of bug 64645 against the current tree".
I can confirm now that backing out the 37415 patch fixes this bug. In fact David is completely right, backing out only the fragment he cited is enough to make it work again (the checking changes two more fragments of two files). In fact the changes are depedent on each other backing them one by one served only the testing of this bug. That checkin was needed to fix some Mac bugs. I am in no position to say if backing this out will regress something on Macs.
Applying patch 39455 fixes this bug. I am not sure about possible Mac regression (bug 76085 nas bug 64645). Adding Peter Lubczynski to CC: as my diff changes his patch 37415.
Keywords: patch
So what's the behavior now? http://www.harrypotter.com and http://www.foxnews.com http://astrology.yahoo.com ...seem to display fine for me working with the trunk tip, Win32. Is there a better, simple testcase? Judging by the date opened, keywords, votes, and cc list, looks like this is quite important but , Karnaze is on vacation. cc:ing more people... :) Jacek, We had to back out the patch from bug 64645 as it was causing some major regressions. What does your patch do?
Keywords: patch
Priority: -- → P1
Better testcase.....but I only see an ASSERTION and an incorrectly placed OBJECT. Jacek, can you better explain the bug here as what I see is not what the screenshot claims: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38603 I will try your patch.
From initial testing, your patch does seem to fix the offset flash addvertisement at Yahoo but since I don't see any of the other problems, I'm not a good tester. I tried it on the Mac and none of the regressions came back. Some of those regressions were awfull painting at disney.com while scrolling on the Mac and can't type in flash text fields like at emailtrack.com on the Mac and the default plugin stopped working. There were others I believe, on all platforms. The patch also does not seem to effect David Einstein's strip down case: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=38660 ..although I see the flash animatin in IE but not in the trunk (with or without your patch). Has this bug been fixed? I don't understand why they all work for me.
I still see the extremely wide page at www.foxnews.com, using the win32 build from the 2001062020 directory, so I don't think this is fixed.
Okay, I now see that the left toolbar is missing from http://www.foxnews.com, but the patch does not fix that. :(
Peter: My patch makes the Reflow return NS_OK always, like it did before your patch. It seems thet otherwise we are in trouble. Read the 6/18 comment by David Einstein - it says it all. Ive just tested his hypothesis and made it into a patch. All the testcases WFM with this patch, including every one you mentioned. BTW, why you have you removed the patch keyword? I used it strictly in accordance with the rules: "Tracks contributions from the net community. Fixes/patches found by individuals without direct code check in privileges." I am such an individual ;-) restoring.
Keywords: patch
Try http://www.foxnews.com/story/0,2933,26760,00.html wit and without the patch. Without it there is no article body visible when you vie the page, just blue space. Like in most other testcases of this bug the text body lands 10 (or so) screen width to the right.
Sorry about the patch keyword. I don't know what happened to it????? But working with the tip, I don't see any difference with or without your patch. Is my build playing tricks on me? Perhaps it's time to take a break....reassign to myself. I recall a bug like this and I believe that your patch does the right thing. But, we need a reproducable testcase in order to check this in. Andrei, Shrir, do you see this? I thought Karnaze just completely backed out the changes from bug 64645 instead of using my patch, I will check bonsai. They were all Karnaze's patches in that bug anyway.
Assignee: karnaze → peterlubczynski
The only patch checked in with bug 76085 was your patch 39455, checked in 6/12 19:42. The full text of the bonsai comment was "bug 76085 - remove the effects of the patch of bug 64645 against the current tree. a=asa, sr=attinasi, r=peterl" but on bug 76085 page karnaze says "The 1st attachment has been checked in and undoes the patch in bug 64645". Maybe he checked in not the patch he meant? Anyway, the thing on the tree is certainly your patch.
The URL I gave before now works fine (Build 2004), but this one is still faulty: http://www.matrox.com/mga/home1.htm
The new Giacomo's link also WFM with the patch applied (as expected). It would be very helpful if someone else build Mozilla with the patch and confirmed whether it works.
i see this also often at http://leela.com.ar , but not everytime
Now I remember the problem very similar to this that Karnaze helped with. It only happened "sometimes" and the page was extra long. The bug was about maxelementsize not being initilized correctly (that's why it works in my debug build). There is an assertion about block frames that comes up with all of these testcases. However, just testing: http://www.matrox.com/mga/home1.htm http://leela.com.ar seemed to work just fine WITHOUT your patch.
Uhm, sorry for the spam but here's another example url: http://www.tpub.com/home.htm
Peter: I see the very wide pages with the two URLs. However, I do not see it at some pages where other people see it. Seems the bug is not 100% reproductible. But you must admit it is certainly real enough with that number of dups.
I see this behavior with W2k build 2001062004
Taking over.....I recall Karanze saying something about the return value so I think this is the right thing to do. I will verify with an optmized build now being created.
r=peterl on patch. Yes....without the patch http://www.foxnews.com is pretty bad. This is a variable-not-initilized problem as it only reproduceable in non-debug builds.
Whiteboard: critical for 0.9.2 → critical for 0.9.2[seeking reviews]
hmm .. its very strange on http://leela.com.ar/fashion/index.shtml firstly the page is very wide... i reload and all is ok.. strange things are going on..
See bug 72427 for a similar problem with the http://www.harrypotter.com page. I've attached a patch, similar to the one in this bug, that I think fixes it, if someone would like to review.
I do not see the Harry Potter problem from bug 72427 with the most recent Win32 build, while I see all the other testcases. For me, this proves they are separate bugs as some people see one but not the other (both ways).
Whiteboard: critical for 0.9.2[seeking reviews] → critical for 0.9.2 [seeking reviews: r=peterl, needs sr=]
*** Bug 87158 has been marked as a duplicate of this bug. ***
Reassign--->Jacek
Assignee: peterlubczynski → piskozub
Target Milestone: --- → mozilla0.9.2
I think we need to reset rv to the value returned by CantRenderReplacedElement instead... We don't want to lose that result. How about: Index: nsObjectFrame.cpp =================================================================== RCS file: /cvsroot/mozilla/layout/html/base/src/nsObjectFrame.cpp,v retrieving revision 1.224 diff -u -r1.224 nsObjectFrame.cpp --- nsObjectFrame.cpp 2001/06/19 22:39:39 1.224 +++ nsObjectFrame.cpp 2001/06/21 06:05:56 @@ -910,6 +910,7 @@ if (NS_FAILED(rv)) { // if we got an error, we'll check for alternative content with // CantRenderReplacedElement() nsIPresShell* presShell; aPresContext->GetShell(&presShell); - presShell->CantRenderReplacedElement(aPresContext, this); + rv = presShell->CantRenderReplacedElement(aPresContext, this);
Marc: I'll check your suggestion ASAP (this afternoon). My patch simply reverts the situation from before patch 39455 which caused this regression (I do agree that forcing rv=NS_OK looks strange). Peter sees no regression with this patch in the problems he fixed with his 39455 patch so it does not collide with what he wanted to achieve.
The change Marc proposes fixes the bug as nicely as the first patch. This means probably we need r= again. Changing the whiteboard.
Whiteboard: critical for 0.9.2 [seeking reviews: r=peterl, needs sr=] → critical for 0.9.2 [seeking reviews: needs r=, sr= for the new patch]
*** Bug 87186 has been marked as a duplicate of this bug. ***
I like the new patch, r=peterl
Whiteboard: critical for 0.9.2 [seeking reviews: needs r=, sr= for the new patch] → critical for 0.9.2, r=peterl, needs sr= for patch 39595
The attached patch looks bogus, it just adds one more call to rv=shell->CantRenderReplacedElement() after the one that's already there, the previous one should be removed, as it was in attinasi's comment in this bug, no? sr=jst for the patch in attinasi's comment :-)
sr=hyatt
Attached patch The final patch (thx jst) (deleted) — Splinter Review
Jst, thanks for noticing my stupid omission! Could someone with the right powers check patch 39636 (r=peterl, sr=jst,hyatt) for me? Without the permission, I'm a poor replacement for a bug owner :-(
Whiteboard: critical for 0.9.2, r=peterl, needs sr= for patch 39595 → critical for 0.9.2, r=peterl, sr=jst,hyatt for patch 39636
I'll check it in, but it still needs drivers@mozilla.org approval. Jacek, could you send mail asking for approval?
a=blizzard on behalf of drivers for 0.9.2
Whiteboard: critical for 0.9.2, r=peterl, sr=jst,hyatt for patch 39636 → critical for 0.9.2, r=peterl, sr=jst,hyatt for patch 39636, a=blizzard
I misinterpreted the recent Blizzard's message as meaning we do not need a= from now on. Now I see that it's only after 0.9.2 branches. It makes sense. I've sent the message... and received a= a few seconds before that :-) Thanks!
Whiteboard: critical for 0.9.2, r=peterl, sr=jst,hyatt for patch 39636, a=blizzard → critical for 0.9.2, r=peterl, sr=jst,hyatt, needs a= for patch 39636
Whiteboard: critical for 0.9.2, r=peterl, sr=jst,hyatt, needs a= for patch 39636 → critical for 0.9.2, r=peterl, sr=jst,hyatt,a=blizzard for patch 39636
Patch checked in, marking FIXED.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
current CVS build, linux: Doesn't quite appear fixed: http://www.digitoday.no/dtno.nsf/pub/te20010622095039_pcs_72599679 Page with a small flash commercial/banner becomes 195 "pages" wide.
R.K.Aa: Your testcase not this bug. There is no <object> in the page source - compare to the bug summary. Niether it is bg 72427 as there is no Flash. You may have hit another similar problem. Consider filing a new bug.
Consider reloading the page. The flash "banner" will appear upper right corner. A Compaq commercial with heading "Golden Offer". I wasn't aware bug 72427 had been reopened though - that should cover it.
So I have the first testcase of bug 72427 can I can actually see. Please post the testcase on that bug's page.
*** Bug 87574 has been marked as a duplicate of this bug. ***
Verified on 2001-08-08-Trunk build on WinNT Above url and the attached sites load fine.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: