5855 – REGRESSION: revert SGML comment parsing fix (comment parsing causes most of usbank.com page to be missing)

RESOLVED FIXED Bug 5855

REGRESSION: revert SGML comment parsing fix (comment parsing causes most of usbank.com page to be missing)

https://bugs.webkit.org/show_bug.cgi?id=5855

Summary REGRESSION: revert SGML comment parsing fix (comment parsing causes most of u...

Ken Sayward

Reported 2005-11-28 13:54:42 PST

This bank website 'should' have a global navigation area around the initial login form. Only the core login form is rendered in Safari. Compare to the correct rendering in Opera or IEWin 5.5sp2. There are further errors deeper within the site, affecting core functionality, but may be related to the same issue. This is the easiest to reproduce without an account with the bank. In a local copy of the page, if I remove the <!doctype ... > tag, the page renders perfectly. This problem did not exist prior to the Safari build prior to OS 10.4.3 update. Latest nightly build as of 10/27 still has the problem. Page renders incorrectly in Camino 0.8.4 and Firefox 1.5rc3 as well, but renders correctly with Opera 8.02 (2148)

Attachments
Screenshot of what the page should look like (60.83 KB, image/png) 2005-11-28 14:03 PST, Ken Sayward	no flags	Details
Screenshot of Safari (incorrect) rendering (67.54 KB, image/png) 2005-11-28 14:04 PST, Ken Sayward	no flags	Details
Minimal Test Case (538 bytes, text/html) 2006-02-11 01:49 PST, Ken Sayward	no flags	Details
Screenshot of minimal test page in Safari 1.3.2 (118.11 KB, image/jpeg) 2006-03-20 10:10 PST, Michael Dreimiller	no flags	Details
revert SGML comment parsing fix (27.25 KB, patch) 2006-04-27 22:04 PDT, Alexey Proskuryakov	hyatt: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Ken Sayward

Comment 1 2005-11-28 14:03:34 PST

Created attachment 4837 [details] Screenshot of what the page should look like Correct Rendering was from Opera.

Ken Sayward

Comment 2 2005-11-28 14:04:30 PST

Created attachment 4838 [details] Screenshot of Safari (incorrect) rendering

Thomas Deniau

Comment 3 2006-02-10 02:14:56 PST

The DOCTYPE used is <!doctype html public "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3c.org/TR/html14/loose.dtd">, which doesn't exist. (the correct URL would be "http://www.w3.org/TR/html4/loose.dtd"). So this site is trying to use a non-existent version of HTML ; I'd classify that as an evangelism bug. Also note that Firefox's rendering (1.5.0.1) is the same as Safari's.

Ken Sayward

Comment 4 2006-02-10 03:14:35 PST

(In reply to comment #3) > The DOCTYPE used is <!doctype html public "-//W3C//DTD HTML 4.01 > Transitional//EN" "http://www.w3c.org/TR/html14/loose.dtd">, which doesn't > exist. (the correct URL would be "http://www.w3.org/TR/html4/loose.dtd"). > > So this site is trying to use a non-existent version of HTML ; I'd classify > that as an evangelism bug. Also note that Firefox's rendering (1.5.0.1) is the > same as Safari's. > Yes, there also appears to be a problem related to the way the comments are included (long ----- seperators) ... attempts to get them to fix this got the usual "sorry, we only code for IEWin" ... unfortunate that IEWin actually renders the page as intended. So, I guess I have to keep Opera around for this site, as it's the only Mac browser I've seen that works with this.

Alexey Proskuryakov

Comment 5 2006-02-10 14:51:31 PST

See also: http://ln.hixie.ch/?start=1137799947

Thomas Deniau

Comment 6 2006-02-10 15:00:58 PST

Replacing the DOCTYPE with a real HTML4 Transitional DOCTYPE doesn't fix the problem, which means that this is indeed a regression which appeared after the acid2 comments parsing fix (removing the DOCTYPE makes Safari switch to quirks mode, where that fix is not applied).

Ian 'Hixie' Hickson

Comment 7 2006-02-10 15:57:43 PST

Note that SGML comment parsing is gone from HTML5. See: http://ln.hixie.ch/?start=1137799947&count=1 Comment parsing in strict mode in HTML5 is defined in: http://whatwg.org/specs/web-apps/current-work/#parsing

Ken Sayward

Comment 8 2006-02-11 01:10:00 PST

Simply removing all of the comment lines where the number of '-' chars are > 2, the page renders correctly, so I'm assuming that the issue is in the comment parsing. I will work on a reduction to get a small test case for this.

Ken Sayward

Comment 9 2006-02-11 01:49:07 PST

Created attachment 6406 [details] Minimal Test Case Small test case showing missing rendering after a malformed comment line (I think ;-). Should display 2 lines of text. the second line does not render when the comment is left in the code. Remove the comment, and the text renders fine. Interestingly, if I remove the SCRIPT line in the HEAD section, the page renders correctly as well. What the heck is that all about??

Alice Liu

Comment 10 2006-03-20 08:38:47 PST

I don't think this is a regression. downgrading priority. Ping me if you find out that it is. This was also seen at http://www.clintonlibrary.gov/. <rdar://problem/4464323>

Michael Dreimiller

Comment 11 2006-03-20 10:10:34 PST

Created attachment 7191 [details] Screenshot of minimal test page in Safari 1.3.2 Here is a screenshot of Safari 1.3.2 (Panther 10.3.9) rendering the minimal test page that Ken submitted . The test page renders correctly. So, it appears that the bug is a regression.

Alexey Proskuryakov

Comment 12 2006-03-20 21:46:02 PST

I can also confirm that this is a regression since 10.3.7 (that's what I have installed besides 10.4.5). I don't think there is any reason to doubt the reporter's words that it happened between 10.4.2 and 10.4.3. Please note that the usbank.com page has changed, and no longer has this problem.

Ken Sayward

Comment 13 2006-03-20 21:59:29 PST

Alexey is correct; the USBank home page has been fixed for proper commenting. However, deeper within that site, there are several pages with the problem still there (yes, I've let them know which ones ;-). The minimal test case is still valid, and correctly represents the problem on the USBank site.

Alexey Proskuryakov

Comment 14 2006-04-27 22:04:37 PDT

Created attachment 8013 [details] revert SGML comment parsing fix This patch reverts one change made for acid2, <http://weblogs.mozillazine.org/hyatt/acid6.txt>. The local copies of acid2 are updated to the current version (no SGML comment parsing, some additional whitespace); fast/parser/comments.html results now match WinIE.

Alexey Proskuryakov

Comment 15 2006-04-27 22:06:25 PDT