Last Comment Bug 193257 - AIM Today page renders incorrectly
: AIM Today page renders incorrectly
Status: RESOLVED FIXED
[adt2]
: testcase, topembed+
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Windows 2000
: P1 normal (vote)
: mozilla1.5alpha
Assigned To: kinmoz
: Madhur Bhatia
Mentors:
http://aimtoday.aol.com/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-02-13 16:18 PST by kinmoz
Modified: 2003-06-05 13:53 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
What the page should look like. (40.53 KB, image/gif)
2003-02-13 16:20 PST, kinmoz
no flags Details
What the page looks like with bad layout. (41.11 KB, image/gif)
2003-02-13 16:22 PST, kinmoz
no flags Details
Minimal Test Case (242 bytes, text/html)
2003-02-14 13:57 PST, kinmoz
no flags Details
JS Test Case (792 bytes, text/html)
2003-02-17 08:14 PST, kinmoz
no flags Details
Patch Rev 1 (1.67 KB, patch)
2003-02-18 10:33 PST, kinmoz
bernd_mozilla: review+
dbaron: superreview+
Details | Diff | Splinter Review

Description kinmoz 2003-02-13 16:18:15 PST
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 2003-02-13 16:20:25 PST
Created attachment 114380 [details]
What the page should look like.
Comment 2 kinmoz 2003-02-13 16:22:00 PST
Created attachment 114381 [details]
What the page looks like with bad layout.
Comment 3 Darin Fisher 2003-02-13 16:48:34 PST
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] 2003-02-13 17:55:31 PST
wfm with win2k build 20030211..
Comment 5 kinmoz 2003-02-14 08:12:21 PST
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 2003-02-14 08:24:50 PST
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 2003-02-14 08:29:36 PST
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) 2003-02-14 11:44:47 PST
nsbeta1+
Comment 9 kinmoz 2003-02-14 13:57:07 PST
Created attachment 114485 [details]
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 2003-02-17 08:10:18 PST
Taking for now.
Comment 11 kinmoz 2003-02-17 08:14:31 PST
Created attachment 114668 [details]
JS Test Case

Here's a JS test case that recreates the problem with a press of a button.
Comment 12 kinmoz 2003-02-18 10:33:32 PST
Created attachment 114803 [details] [diff] [review]
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 2003-02-18 13:13:01 PST
Cc'ing bernd and jkeiser for comments on Patch Rev 1.
Comment 14 Bernd 2003-02-20 12:12:34 PST
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 2003-05-09 10:44:33 PDT
adt: nsbeta1+/adt2
Comment 16 saari (gone) 2003-05-19 10:30:03 PDT
CC'ing dbaron and topembed+
Comment 17 kinmoz 2003-06-05 10:46:39 PDT
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 2003-06-05 13:53:40 PDT
I filed bug 208462 to track the concerns bernd raised in comment 14.

Note You need to log in before you can comment on or make changes to this bug.