These pages aren't laying out right; extra rows are constructed in tables. Why: Fixes that went into the parsing engine 2 weeks ago (to fix Gamespot, bloomberg) revealed another problem here. This has to do with this sequence: <table><tr><td><li><table> The nested table should be permitted, but isn't. The cause is that tables aren't part of any element group (like block or inline) though they act that way. The code that was added to fix "block" cases fail to apply here. The fix is to add a test for *table_elements* (td, tr, th, etc) to this routine. Here's the patch: Index: CNavDTD.cpp =================================================================== RCS file: /cvsroot/mozilla/htmlparser/src/CNavDTD.cpp,v retrieving revision 3.322 diff -r3.322 CNavDTD.cpp 1004a1005,1007 > static eHTMLTags gTableElements[]={ eHTMLTag_table,eHTMLTag_tbody, > eHTMLTag_tr,eHTMLTag_td,eHTMLTag_th,eHTMLTag_tfoot, eHTMLTag_thead); > 1016a1020,1024 > else if(FindTagInSet(theParentTag,gTableElements,sizeof(gTableElemen ts)/sizeof(eHTMLTag_unknown))){ > result=PR_TRUE; > break; > }

Rickg, could you please post a diff with -uw flags? Thanx

PDT: This is essential. Fixes that went in 2 weeks ago have unconvered other shortcomings in the way we handled bad markup (that is very common). These pages worked fine when I landed, but changes to the site reveal the problem. Given the common issue and the easy fix, I recommend that we fix this problem. r=attinasi, sr=buster.

Here's the patch again, in the form harish wanted: Index: CNavDTD.cpp =================================================================== RCS file: /cvsroot/mozilla/htmlparser/src/CNavDTD.cpp,v retrieving revision 3.322 diff -w -u -r3.322 CNavDTD.cpp --- CNavDTD.cpp 2000/10/13 01:16:56 3.322 +++ CNavDTD.cpp 2000/10/18 19:56:24 @@ -1002,6 +1002,9 @@ result=PR_FALSE; + static eHTMLTags gTableElements[]={ eHTMLTag_table,eHTMLTag_tbody,eHTML Tag_tr, + eHTMLTag_td,eHTMLTag_th,eHTMLTag_tf oot,eHTMLTag_thead}; + PRInt32 theIndex=theCount-1; while(theChildIndex<theIndex) { eHTMLTags theParentTag=aContext.TagAt(theIndex--); @@ -1014,6 +1017,11 @@ break; } } + else if(FindTagInSet(theParentTag,gTableElements,sizeof(gTableElement s)/sizeof(eHTMLTag_unknown))){ + result=PR_TRUE; + break; + } + } } }

Nominating this for rtm. Both sites are written up for bad layout in bugs #57156 and #56754 (the two are both nominated for rtm).

The hard part of this bug is that it deals with a particular kind of bad markup. Sites change their markup all the time -- which explains why we've seen this problem come and go over the last year. The code I added 2 weeks ago was an attempt to constrain the problem, and the pages here show a sub-case which I missed (because, again -- table elements aren't "block"). Once harish and I get through the regressions and top 100, I hope the PDT will approve.

*** Bug 56994 has been marked as a duplicate of this bug. ***

Went through the patch and have asked rickg to make a minor change. Other than that everything looks fine. I walked through the top 100 sites, with the patch, and saw no problems. majorleaguebaseball.com and pgtour.com now render correctly. r=harishd

Actually, there's a bug related to Accuweather that has nothing to do with this issue. We occasionally get duplicate banner ads in this JS generated section -- and Harish and I are working on that issue seperately.

Accuweather doesn't sound like a bug after all. When viewed thro' mozilla the page renders correctly however when viewed thro' viewer we get a double banner on the top. The reason behind this is that our JS engine, I think, failed to identify viewer as Netscape 6, and therefore doesn't trigger a correct condition (if {)in the document.

No the accuweather site was hosed in mozilla, that is how I first noticed it. Look at my bug 56994. And try link: http://www.accuweather.com/adcbin/local_index?thisZip=92129&btnZip=Go&nav=home with and without the fix. My bug may be a dup, because basically I had narrowed the problem down to rick's check in on the morning of Oct 12th.

Two seperate problems have been discussed here. The first has to do with how we handle bad markup, and was the original point of this bug (shown on majorleaguebaseball.com). The second issue that harish was discussing was the result of JS that did browser sniffing and sent buggy script to viewer (which is why Harish called it "not a bug"). We can safely ignore the second issue.

here's the formal patch: --- CNavDTD.cpp 2000/10/15 17:30:07 3.323 +++ CNavDTD.cpp 2000/10/19 01:04:54 @@ -1027,6 +1027,8 @@ result=PR_FALSE; + static eHTMLTags gTableElements[]={eHTMLTag_td,eHTMLTag_th}; + PRInt32 theIndex=theCount-1; while(theChildIndex<theIndex) { eHTMLTags theParentTag=aContext.TagAt(theIndex--); @@ -1038,6 +1040,10 @@ result=PR_TRUE; break; } + } + else if(FindTagInSet(theParentTag,gTableElements,sizeof(gTableElemen s)/sizeof(eHTMLTag_unknown))){ + result=PR_TRUE; + break; } } }

PDT says rtm++, please land on branch ASAP.

As promised, I'll have buster sign his approval before landing. With luck it will land tonight.

sr=buster

*** Bug 56754 has been marked as a duplicate of this bug. ***

*** Bug 57156 has been marked as a duplicate of this bug. ***

Improvements landed.

Verified fixed with 10/19 branch builds on Win98, Win2000, Mac8.6, Linux6.2 (version 2.2). Adding vtrunk keyword.

Verified with 10/23-25 trunk builds on WinNT, Mac9, Linux6.2 Removing vtrunk keyword. Marking as VERIFIED