237734 - Failure to display content with certain combination of background-attachment: fixed, vertical-align, and <a>

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Ben Sizer]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040311 Firefox/0.8 StumbleUpon/1.9 (BlueFyre)
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040311 Firefox/0.8 StumbleUpon/1.9 (BlueFyre)

http://forums.mozillazine.org/viewtopic.php?t=61496

On official 0.8, it renders fine, showing some text, a linked image, then some
more text.
On builds of 0.8 made in the last few days, nothing appears. Sometimes it
appears very briefly and then disappears when the page stops rendering. Yet the
text is considered to be there, as the cursor changes when you hover over it.

I found 3 ways of getting it to work on the newer builds, which implies a
complex relationship:
- commenting out the background-attachment: fixed part of the 'broken' class's style
- changing vertical-align: on the image from bottom to default (top seemed to be
broken too)
- removing the <a> tag surrounding the image. 

Reproducible: Always
Steps to Reproduce:
1. Open test case at http://homepage.ntlworld.com/ben.sizer/testmoz/friends.htm

Actual Results:  
Nothing visible in browser window.

Expected Results:  
Displayed some text, with a hyperlinked graphic in the middle.

Also confirmed on NT 5.1 (XP?) and Linux.  (See mozillazine link above.)

Some more complex variations of this problem (eg. where the block element has an
absolute size) seem to continue to show the fixed background but the text is
invisible. I expect this is a variation on the same bug but I don't have a
minimal test case for that.

Comment 1

[PikeUK]

Do you know the exact day it regressed?

Comment 2

[Ben Sizer]

I'm not sure, but it was probably 2 or maybe even 3 weeks ago. I was seeing the
symptoms for quite a while before I took the time to isolate the problem and
generate the test case.

Comment 3

[PikeUK]

This regressed between 2004-02-27 and 2004-02-28 (main changes in that timeframe
are bug 53966, bug 235778 and bug 229371). Backing out the changes from bug
235778 seem to solve the problem.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

The following sequence happens during incremental reflow in
nsTableRowGroupFrame::IR_TargetIsChild:
-- We incrementally reflow the row
  -- which reflows the cell (getting an empty overflow area for the cell, since
the cell defers overflow calculation to until VerticallyAlignChild is called)
  -- and does FinishReflowChild on the cell which sets its view to the overflow
area, which is empty
-- we choose not to call DidResize on the table row, because the height did not
change, here:
http://lxr.mozilla.org/mozilla/source/layout/html/table/src/nsTableRowGroupFrame.cpp#1707

This is bad; we were relying on DidResize to call VerticallyAlignChild to set
the cell's overflow area and resynchronize the view.
-- Result: the view remains empty and the cell disappears.

This patch solves the problem by preserving the old overflow area for the cell
in nsTableCellFrame::Reflow. If incremental reflow skips DidResize, we
basically assume the overflow area hasn't changed.

This is very low risk and clearly better than what we have now --- suitable for
1.7. However I'm not sure it's a complete fix since incremental reflow could
change the overflow area without changing the actual cell height, and I think
in that case we would fail to record the changed overflow area. A full fix
might be to call DidResize whether the cell height changed or not, but that
might be a bit riskier. Let's see what bernd says.

Comment 5

[Bernd]

I don't think we need this patch. Robert the patch that you r/sr'ed in bug
230730 looks more compact and does fix the testcase.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The patch in bug 230730 does NOT fix this bug. I tried it.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We oughta be able to fix this layout regression for 1.7.

Comment 8

[Bernd]

Attachment: patch

This patch is identical to the thing that I just checked in, that is used in
VerticalAlignChild. The patch for bug 230730 fixes the url in this bug on
initial load (that should be a lesson for me to use still the viewer.exe :-().
With this patch I can even reload the testcase and the content does not vanish.
Robert could you please retest.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 144941 [details] [diff] [review]
patch

Indeed, this is the right fix. bernd, you're a hero!

Comment 10

[Bernd]

taking for checkin.

Comment 11

[chris hofmann]

Comment on attachment 144941 [details] [diff] [review]
patch

a=chofmann for 1.7

Comment 12

[Bernd]

fix checked in