193257 - AIM Today page renders incorrectly

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect, P1)

Description

[kinmoz]

When I load http://aimtoday.aol.com in my win32 TRUNK debug build, it renders
incorrectly most of the time for me.

I'll attach 2 screenshots showing what I'm seeing, and how it should look.

At the outer most level, the  page is layed out in a table with 2 rows and 1
column with a dimension of width=550 and height=360.

When things render correctly, the first row appears, it seems to take up the
entire 550x360 size, and then the 2nd row appears below it (at this point the
table is probably twice as high as it should be), and then eventually everything
gets resized to fit within the 550x360.

When things don't work it's as if the last resize mentioned above is somehow missed.

This bug is timing related, and to make things more complicated, the page loads
an external style sheet, lots of images, and uses JS to write content into the
content tree.

Comment 1

[kinmoz]

Attachment: What the page should look like.

Comment 2

[kinmoz]

Attachment: What the page looks like with bad layout.

Comment 3

[Darin Fisher]

kin: how old is your trunk build?  i can't repro the problem using trunk
builds... windows (2003021208) and linux (2003021108).

Comment 4

[Matthias Versen [:Matti]]

wfm with win2k build 20030211..

Comment 5

[kinmoz]

My TRUNK debug tree is from the 02/10/03, as I said it's timing related ... it
might also help that I'm sucking bits over a 56k modem.

Comment 6

[kinmoz]

Another observation ... I seem to hit this assertion whenever I see the problem:

###!!! ASSERTION: attribute encountered -- this shouldn't happen unless the attr
ibute was not part of a start tag!: 'Error', file x:/mozilla/htmlparser/src/CNav
DTD.cpp, line 2265
Break: at file x:/mozilla/htmlparser/src/CNavDTD.cpp, line 2265


Here's the stack:


NTDLL! 77f97704()
nsDebug::Assertion(const char * 0x023630e8, const char * 0x10133390, const char
* 0x023630c0, int 2265) line 280 + 13 bytes
nsDebug::Error(const char * 0x023630e8, const char * 0x023630c0, int 2265) line
463 + 22 bytes
CNavDTD::HandleAttributeToken(CToken * 0x03b64718) line 2265 + 21 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03ce7070, CToken * 0x03b64718, nsIParser
* 0x014c84d0) line 964 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03ce7070, nsIParser * 0x014c84d0,
nsITokenizer * 0x03ce7788, nsITokenObserver * 0x00000000, nsIContentSink *
0x014c9b10) line 519 + 20 bytes
nsParser::BuildModel() line 1904 + 34 bytes
nsParser::ResumeParse(int 1, int 1, int 1) line 1771 + 11 bytes
nsParser::ContinueParsing() line 1388 + 19 bytes
nsParser::HandleParserContinueEvent() line 1448
nsParserContinueEvent::HandleEvent() line 230
HandlePLEvent(nsParserContinueEvent * 0x03afe3f0) line 241
PL_HandleEvent(PLEvent * 0x03afe3f0) line 663 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00dd8118) line 593 + 9 bytes
_md_TimerProc(HWND__ * 0x0001061a, unsigned int 275, unsigned int 0, unsigned
long 5002713) line 964 + 9 bytes
USER32! 77e11d0a()
USER32! 77e11c40()
USER32! 77e11cef()
nsAppShellService::Run(nsAppShellService * const 0x01440a20) line 480
main1(int 2, char * * 0x00262698, nsISupports * 0x00dd1610) line 1273 + 32 bytes
main(int 2, char * * 0x00262698) line 1636 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL

Comment 7

[kinmoz]

Here's a slightly different stack for the assertion:


NTDLL! 77f97704()
nsDebug::Assertion(const char * 0x023630e8, const char * 0x10133390, const char
* 0x023630c0, int 2265) line 280 + 13 bytes
nsDebug::Error(const char * 0x023630e8, const char * 0x023630c0, int 2265) line
463 + 22 bytes
CNavDTD::HandleAttributeToken(CToken * 0x03786888) line 2265 + 21 bytes
CNavDTD::HandleToken(CNavDTD * const 0x03793cb0, CToken * 0x03786888, nsIParser
* 0x034827b8) line 964 + 12 bytes
CNavDTD::BuildModel(CNavDTD * const 0x03793cb0, nsIParser * 0x034827b8,
nsITokenizer * 0x03779710, nsITokenObserver * 

0x00000000, nsIContentSink * 0x036f4748) line 519 + 20 bytes
nsParser::BuildModel() line 1904 + 34 bytes
nsParser::ResumeParse(int 1, int 1, int 1) line 1771 + 11 bytes
nsParser::ContinueParsing() line 1388 + 19 bytes
CSSLoaderImpl::SheetComplete(SheetLoadData * 0x03707000, int 1) line 1778
CSSLoaderImpl::ParseSheet(nsIUnicharInputStream * 0x03731250, SheetLoadData *
0x03707000, int & 1) line 1715
SheetLoadData::OnStreamComplete(SheetLoadData * const 0x03707000,
nsIUnicharStreamLoader * 0x037e89f0, nsISupports * 

0x00000000, unsigned int 0, nsIUnicharInputStream * 0x03731250) line 1120 + 23 bytes
nsUnicharStreamLoader::OnStopRequest(nsUnicharStreamLoader * const 0x037e89f4,
nsIRequest * 0x03792038, nsISupports * 

0x00000000, unsigned int 0) line 189
nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x033b5ab8,
nsIRequest * 0x03792038, nsISupports * 0x00000000, 

unsigned int 0) line 66
nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x03792040, nsIRequest *
0x03722370, nsISupports * 0x00000000, unsigned int 

0) line 2951
nsInputStreamPump::OnStateStop() line 468
nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x03722374,
nsIAsyncInputStream * 0x03524324) line 320 + 11 

bytes
nsInputStreamReadyEvent::EventHandler(PLEvent * 0x037d9adc) line 112
PL_HandleEvent(PLEvent * 0x037d9adc) line 663 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00dd8118) line 593 + 9 bytes
_md_TimerProc(HWND__ * 0x000205e8, unsigned int 275, unsigned int 0, unsigned
long 5613601) line 964 + 9 bytes
USER32! 77e11d0a()
USER32! 77e11c40()
USER32! 77e11cef()
nsAppShellService::Run(nsAppShellService * const 0x01440a20) line 480
main1(int 2, char * * 0x00262698, nsISupports * 0x00dd1610) line 1273 + 32 bytes
main(int 2, char * * 0x00262698) line 1636 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL

Comment 8

[Kevin McCluskey (gone)]

nsbeta1+

Comment 9

[kinmoz]

Attachment: Minimal Test Case

Ok here's the minimal test case I came up with. Most of the time you load this
page, it will render corectly. The only way to see the bug reliably is to put a
breakpoint in IsTimeToNotify() in
mozilla/content/document/src/nsHTMLContentSink.cpp, right at the call to
PR_Now().

What I do is I load the test case in the browser, enable the breakpoint, then
go back into the browser and hit F5 (Reload) ... allow the breakpoint to be hit
3 times, disable it, then continue.

Comment 10

[kinmoz]

Taking for now.

Comment 11

[kinmoz]

Attachment: JS Test Case

Here's a JS test case that recreates the problem with a press of a button.

Comment 12

[kinmoz]

Attachment: Patch Rev 1

Here's a patch that makes things reflow properly.

The JS Test Case (attachment 114668 [details]) actually takes a slightly different
codepath (nsTableRowGroupFrame::InsertFrames()) than is taken when loading the
http://aimtoday.aol.com or Minimal Test Case (attachment 114485 [details]), which both
seem to trigger nsTableRowGroupFrame::AppendFrames() instead, hence the same
change in two places.

I would've posted a JS test case that tested AppendFrames() but I haven't
figured out if that is possible yet since I am already using appendChild in the
JS test case.

Comment 13

[kinmoz]

Cc'ing bernd and jkeiser for comments on Patch Rev 1.

Comment 14

[Bernd]

I think the patch solves the problem but would like to see a softer approach
especially for the AppendFrame case. There might be no softer approach, but I
will not be able to propose something better before the weekend or to see that
there is no better way. Is this strategy thing also needed for fixed layout?
Wouldnt the same problem also appear for rowgroup with specifed heights?

Comment 15

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 16

[saari (gone)]

CC'ing dbaron and topembed+

Comment 17

[kinmoz]

Patch Rev 1 checked into the TRUNK:

  mozilla/layout/html/table/src/nsTableRowGroupFrame.cpp  revision 3.301

I'll need to do some test cases to check the row group height case bernd brings
up above. If it is indeed a problem, I'll open another bug to address it.

Comment 18

[kinmoz]

I filed bug 208462 to track the concerns bernd raised in comment 14.