Closed Bug 128133 Opened 23 years ago Closed 23 years ago

[PATCH]Screen flicker and ghost of flash/iframe content with dhtml

Categories

(Core :: Layout, defect, P2)

x86
Windows 2000
defect

Tracking

()

VERIFIED FIXED
mozilla1.0

People

(Reporter: rsemirag, Assigned: attinasi_layout)

References

()

Details

(Keywords: regression, topembed, Whiteboard: [adt2] [DIG])

Attachments

(6 files)

From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.8) Gecko/20020204 BuildID: 2002022703 (Win2k) A ghost image of flash or iframe content appears ~50-100 pixels above the actual content and flickers in time to dhtml activity. In the above example, while the image is moved across the page. I have another example url (coming soon) of the same behavior involving dhtml menus and an iframe. I have encountered this behavior in Linux and Windows 2000, using the nightly build above, Mozilla 0.9.8 and Netscape 6.2. This has also been seen in Netscape 6.2 on WinNT 4.0. I have verified that this behavior is not in Netscape 6.1, so it is a fairly recent addition to the layout engine. Reproducible: Always Steps to Reproduce: 1.Load this page and wait for the moving dhtml image to start across the page. Actual Results: The flash ghost image flickers 150 pixels above the actual flash Expected Results: Not show the flash ghost image
Confirming on the Feb 27th build (2002-02-27-03) under Windows ME. Doesn't occur on OS X build (2002-03-01-03).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P4
Target Milestone: --- → Future
Iframe example: http://www.sportsline.com/ Same as the flash example, the iframes ghost a few hundred pixels above their actual location when you mouse over the dhtml menus. The iframe content is labeled Brackets Contests and Fantasy/My Leagues.
Attached file Reduces test case of flicker problem (deleted) —
Test case includes two left aligned tables. The second table contains the embed element which references the flash file.
Well, the problem is related to the embed in a floated table. If you remove the align-"left" from the table that contains the <embed> then the flickering / ghosting cruft goes away. To fix this in the code is going to be at least a day's work, possibly more. I need to put in some probes to figure out why the floated table is getting moved around so much - it should not be. If we cannot fix this in the markup (eg. evangelism) then I'll take a crack at it - pease advise.
Status: NEW → ASSIGNED
I have a fix for this. The problem is that the plugin is in a floated element (in my testcase, a div, could be a table as well). The floated element is reflowed whenever the blimp moves. During the reflow, we are first repositioning the view for the floated element's frame to a bogus location (the top of the page, basically) and then after reflowing and placing the floater where we want it, we set the view position again, this time to the correct location. I cannot think of any reason to do the initial view placement - it is going to be a wrong location, and it will have to be moved again later anyway. Stopping the first view placement stops this bug. Now I just need to figure out if that breaks something else...
This patch removes a call to PlaceFrameView that was setting the view position incorrectly. Later, after the frame was reflowed, the view would be moved back to the correct location, but the movement from position A to position B and then back to position A caused the flicker. From the comments in the code it looked like there was some question as to whether or not that call to ::PlaceFrameView should have been made at all - I'm querying dbaron on that since he wrote (rewrote) that code. However, from my tests the position of that view was never correct as a result of that call, so I cannot see what good it was. Also, by moving the view(s) to an intermediate position and then later back to the correct position we subcert the optimizations of the view manager that minimize the work required if a view is moved to the same position that it is already in. Thus we get better performance and no flicker. Now, to figure out what might have broken...
What I needed that for is what I mentioned in bug 86947 comment 31, that is, the bugzilla query page. I forget whether it was the textboxes or the selects, but I remember that some things weren't moving when I resized the bugzilla query page and/or bugzilla bug pages. (At least I think it was resizing rather than incremental loading.)
Thanks David. I tested Bugzilla Query and some bug pages, resizing the window and messing with the listboxes and combo-boxes - no problems. I'll concoct some tests that float those controls and see what I get too, just to be safe. The block regression tests passed (though there are two testcases that crash due to reading free'd memory, but that is in both the baseline and modified build). Admitedly, the block regression tests will not detect the issues with resizing...
I noticed that there is a problem (with testcase and www.sportsline.com) with flickering when resizing the window horizontally as well. That problem is due to the table code erroneously repositioning the views for the floated table before the table has actually been placed, so again the view are positioned to an incorrect intermediate position before being correctly positioned, causing the flicker. This patch, used in addition to the first one (76649), fixes the flickering in the DHTML case and the resizing case, and should make www.sportsline.com happy. It might be a good idea to revisit the whole notion of repositioning views during reflow - I think there is a mixture of assumptions and approaches in the layout code...
CC'ing karnaze since I have now entered Table Land (tm). Also, nominating for NSBETA1 since this is a pretty ugly problem on a pretty popular site.
Keywords: nsbeta1
Priority: P4 → P2
Target Milestone: Future → mozilla1.0
nominating for topembed as sportlines is *the* AOL sports partner.
Keywords: topembed
nsbeta1+.
Keywords: nsbeta1nsbeta1+
BTW: this problem of positioning a floated element's views before it is positioned, and then again after it is positioned, is very general and will impact a lot of sites since left-side navigation areas are very common (though it needs an IFRAME, a plugin, or some other widget-containing element to exhibit the flicker). I'm pretty sure it is a regression from 0.9.4 but I have not narrowed down the milestone where it broke.
Keywords: nsbeta1+nsbeta1
Keywords: nsbeta1nsbeta1+
adt2
Whiteboard: [adt2]
Comment on attachment 76760 [details] [diff] [review] Additional patch: tables need to stop repositioning views during reflow for floated tables r=karnaze
Attachment #76760 - Flags: review+
Comment on attachment 76649 [details] [diff] [review] PATCH: removes unnecessary call to PlaceFrameView from ReflowDirtyLines r=dbaron if you've tested that resizing the bugzilla query page and bugzilla bug pages works as expected
Attachment #76649 - Flags: review+
Yes, I tested the bugzilla case, and a whole bunch of other pages using floated elements, floated plugins, non-floated plugins, and form controls.
Comment on attachment 76649 [details] [diff] [review] PATCH: removes unnecessary call to PlaceFrameView from ReflowDirtyLines sr=waterson
Attachment #76649 - Flags: superreview+
Comment on attachment 76760 [details] [diff] [review] Additional patch: tables need to stop repositioning views during reflow for floated tables sr=waterson
Attachment #76760 - Flags: superreview+
-->adt1.0.0
Keywords: adt1.0.0
Comment on attachment 76649 [details] [diff] [review] PATCH: removes unnecessary call to PlaceFrameView from ReflowDirtyLines a=roc+moz
Attachment #76649 - Flags: approval+
Comment on attachment 76760 [details] [diff] [review] Additional patch: tables need to stop repositioning views during reflow for floated tables a=roc+moz
Attachment #76760 - Flags: approval+
Summary: Screen flicker and ghost of flash/iframe content with dhtml → [PATCH]Screen flicker and ghost of flash/iframe content with dhtml
adt1.0.0+ (on ADT's behalf) for approval for checkin.
Whiteboard: [adt2] → [adt2] [DIG]
Fixes checked into trunk. /cvsroot/mozilla/layout/html/table/src/nsTableOuterFrame.cpp,v <-- nsTableOute rFrame.cpp new revision: 3.220; previous revision: 3.219 /cvsroot/mozilla/layout/html/table/src/nsTableRowFrame.cpp,v <-- nsTableRowFra me.cpp new revision: 3.305; previous revision: 3.304 done /cvsroot/mozilla/layout/html/base/src/nsBlockFrame.cpp,v <-- nsBlockFrame.cpp new revision: 3.495; previous revision: 3.494
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
can this have caused bug 136078?
it caused bug 136080. I backed out this fix and the problem goes away.
will this patch fix (still unconfirmed) bug 119163 as well? i might dup that one then to this...
Bug 136080 was basically the problem I mentioned in comment 8 and comment 17. I find it most noticeable with the cc: field listbox on the bugzilla bug pages (like this page), which often gets misplaced for me on the initial load of the page, and after than "drags" one resize behind when resizing the page.
In addition to causing bug 136080, it (actually on the 1st patch) has caused a problem on http://bugzilla.mozilla.org/show_bug.cgi?id=126160 where the <select> is badly placed. I'm seeing it on initial load, but alexsavulov writes: I can see this in build 2002040803 I also discovered that is reproductible only when refreshing the window (SHIFT + click on refresh) and the window has the width seen in the the jpg image (aprox. 962 px including the scrollbar). when resizing the window, the layout gets fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Well, maybe what I just reported is bug 136080.
Attached file regression testcase (deleted) —
i also found out that if you get the right window width (about 962px) and you slowly resize around that width you'll get the select to be placed wrongly too although another place than when refreshing.
Attached image screenshot (deleted) —
Well I see the problem on bugzilla now :) Backing out until I can figure out why we are sometimes placing the views correctly and sometimes not.
Assignee: attinasi → attinasi_layout
Status: REOPENED → NEW
Backing out the first patch fixes the bugzilla page (bug 136080) and also the original URL. I'm going to back out that patch and leave the table patch in. bug 136078 has nothing to do with this bug (I tested it with my changes backed out and it made no difference).
Status: NEW → ASSIGNED
Blocks: 136255
BlockFrame patch backed out: /cvsroot/mozilla/layout/html/base/src/nsBlockFrame.cpp,v <-- nsBlockFrame.cpp new revision: 3.496; previous revision: 3.495 I left the table patche in because it fixes this bug, and removing just the block frame patch fixes all of the reported regressions. Marking FIXED again.
No longer blocks: 136255
Status: ASSIGNED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Verified on the April 09th build (2002-04-09-03).
Status: RESOLVED → VERIFIED
Well, the page doesn't flicker at the URL anymore, but btestcase #2 ( http://bugzilla.mozilla.org/attachment.cgi?id=76654&action=view ) still displays a "ghost" flicker every time the the text is clicked (to move the image). Also, check out the menu bar at brainjar.com ( http://www.brainjar.com/dhtml/menubar/demo.html ). After the menu is clicked on to drop down a menu, all mouseovers produce a flicker in the upper left hand corner of the window. does this bug cover these cases? If so, it should be reopened. Jake
The testcase that is still broken (http://bugzilla.mozilla.org/attachment.cgi?id=76654&action=view) needed the block frame patch that was backed out. I'd prefer a new bug on that specific issue rather than expanding this one. Also, if there are any real sites that show this, that would be good to note.
I reported the bug about fixing the block frame flickering as bug 136549. Please fix this one especially since you already have an existing patch. It really makes Mozilla's dhtml look shoddy! Jake
Keywords: fixed1.0.0
Verified on Windows Me May 19th branch (2002-05-19-05).
Keywords: verified1.0.0
*** Bug 56517 has been marked as a duplicate of this bug. ***
Looks like the sort of thing that roc would have an opinion on. CC:ing.
Whoops. Meant to draw attention to bug 136549 instead of this closed one.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: