User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2b4) Gecko/20091124 Firefox/3.6b4 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2b4) Gecko/20091124 Firefox/3.6b4 (.NET CLR 3.5.30729) position fixed visibility hidden table causes flickering when window scrolls. Reproducible: Always Steps to Reproduce: 1.scroll 2.see glitches. 3.back to 1 Actual Results: i see flickering (rendering delay) around hidden table. Expected Results: position fixed visibility hidden elements should not affect rendering result.

please see this example. hope the problem reproduces with your firefox.

fixed and simplified html. so the page layout would be the same on InternetExplorer. anywayz, the problem also reproduces with Firefox3.5.5.

The out-of-sync-ness is very easy to see when smooth scrolling is enabled. I wonder if display lists could optimize away the whole position:fixed element?

On Mac I actually see a permanent rendering artifact.

Oh, that's probably a regression from my patch for bug 530686.

Ok, I found the regression from bug 530686 and submitted a new patch there. This bug isn't visible on Mac, but it is on Linux.

Yep, I saw it on Linux (and the original report is on Windows).

The scroll analysis has an nsDisplayTableBorderBackground in it, because of this comment: http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableFrame.cpp#1338 I guess we should just traverse the frames for the table and avoid adding nsDisplayTableBorderBackground if they're all hidden.

Avoid creating an nsDisplayTableBorderBackground if every part of the table is hidden.

Comment on attachment 415305 [details] [diff] [review] fix >- NS_ASSERTION(currentItem, "No current table item!"); >- currentItem->UpdateForFrameBackground(aFrame); >+ // currentItem may be null if none of the table parts are visible >+ if (currentItem) { >+ currentItem->UpdateForFrameBackground(aFrame); >+ } I don't like this change. It seems that: * if we're doing appropriate visibility checks, we wouldn't need this change at all * if we actually do need this change, we might notify an nsDisplayTableItem for a containing visible table instead, which seems bad (I also wonder whether table *cells* should clear the current table item so it's no longer set while processing their descendants.) >@@ -1307,16 +1309,32 @@ nsTableFrame::DisplayGenericTablePart(ns > // before everything else (cells and their blocks) > separatedCollection.BorderBackground()->Sort(aBuilder, CompareByTablePartRank, nsnull); > separatedCollection.MoveTo(aLists); > } > > return aFrame->DisplayOutline(aBuilder, aLists); > } > >+static PRBool >+AnyTablePartVisible(nsIFrame* aFrame) >+{ Can you assert at the start of this that the frame is some table part? (Perhaps through aFrame->GetStyleDisplay->mDisplay ?)

(In reply to comment #10) > * if we're doing appropriate visibility checks, we wouldn't need this change > at all I'm not sure what you mean. I still want to call DisplayGenericTablePart even if every part of the table is hidden, otherwise we'd need a different code path that recursively calls BuildDisplayListForChild on the children. And if we call DisplayGenericTablePart from nsTableFrame::BuildDisplayList and pass in a null aDisplayItem, we'll crash without this change. > * if we actually do need this change, we might notify an nsDisplayTableItem > for a containing visible table instead, which seems bad Indeed... > (I also wonder whether table *cells* should clear the current table item so > it's no longer set while processing their descendants.) That's a good idea, I'll do that. > Can you assert at the start of this that the frame is some table part? Sure.

I meant that that the call to currentItem->UpdateForFrameBackground ought to be within a check that the frame is visible (and perhaps some other code in that function?), and if it were, you wouldn't need to null-check currentItem. Also, your assertion should allow column groups and columns.

You're right again!

I can't seem to write a testcase where a box-shadow shows up on a table-related frame with visibility:hidden, though. In what cases would we reach and then fail that IsVisibleForPainting check?

I'm not sure what you're asking. On trunk, a box-shadow on a visibility:hidden table part won't show up because hasBoxShadow checks IsVisibleForPainting. Here is a testcase that will reach and fail the IsVisibleForPainting check in "fix v3". <table border="1" style="visibility:hidden"> <tr style="visibility:visible"><td>Hello <tr><td>Kitty</table> The check will fail when DisplayGenericTablePart is called for the hidden table, table-row-groups, and table-row.

Comment on attachment 415568 [details] [diff] [review] fix v3 Oops, missed that check. Sorry. RequestSpecialHeightReflow should still assert if it's given a colgroup or col frame; you could either check for those specifically or just undo the change. r=dbaron with that

Landed: http://hg.mozilla.org/mozilla-central/rev/9af5c4367b00 and then addressed the review comment in: http://hg.mozilla.org/mozilla-central/rev/8bc6e7e44946 This will need to land on 1.9.2 because it blocks bug 530686. (In other words, it's basically the first two thirds of the patch to bug 530686.)

The above patch caused large numbers of assertions during reftest because we're finding nsScrollbarFrames. This fixes those assertions, and I think it's the right thing to do.

Assertion fix landed: http://hg.mozilla.org/mozilla-central/rev/89a4f5a6525a

assertion fix landed: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/79f925475a39