RESOLVED FIXED 12794
REGRESSION: TripTik planner at aaa.com never finishes loading due to unclosed canvas tag
https://bugs.webkit.org/show_bug.cgi?id=12794
Summary REGRESSION: TripTik planner at aaa.com never finishes loading due to unclosed...
Simon Pride
Reported 2007-02-16 13:57:07 PST
Using the interactive TripTik planner, the applet in the left frame never finishes loading. Works fine in release Safari 2.0.4. Sample URL (contains session ID cruft): http://ww1.aaa.com/scripts/WebObjects.dll/AAAOnline.woa/4014/wo/VULCxz1Spe682RojJvOSHw/0.9.13.8.3.0.1.2.2.1.1.0.1.1.0.0
Attachments
test case (87 bytes, text/html)
2007-02-24 00:33 PST, Alexey Proskuryakov
no flags
patch with change log and test case (36.87 KB, patch)
2007-03-10 10:10 PST, Darin Adler
mjs: review-
revised patch, with change log and regression tests (50.56 KB, patch)
2007-03-13 08:58 PDT, Darin Adler
mjs: review+
Alexey Proskuryakov
Comment 1 2007-02-16 22:54:08 PST
Confirmed as a regression. There is a "maximum call stack exceeded" error in the console. Steps to reproduce: 1. Open http://www.aaa.com 2. Enter some U.S. zip code (such as 95014) 3. Click "TripTik(R) Travel Planner" link under Maps & Directions.
Alexey Proskuryakov
Comment 2 2007-02-23 14:08:16 PST
The "maximum call stack exceeded" error is red herring - it is caused by unreliable error logging code. The actual problem appears to be a result of an unclosed <canvas> tag: ... <canvas ID="dRouteCanvas" width=2000 height=2000 /> </DIV>
Darin Adler
Comment 3 2007-02-23 14:09:40 PST
Does this fail with Firefox too? I ask because I believe Firefox would also consider this an unclosed <canvas> tag.
Simon Pride
Comment 4 2007-02-23 14:23:39 PST
> Does this fail with Firefox too? I ask because I believe Firefox would also consider this an unclosed <canvas> tag. No, it works fine in the current version of Firefox (2.0.0.1 on PowerPC).
Darin Adler
Comment 5 2007-02-23 14:32:43 PST
We should figure out why WebKit's rule doesn't match Firefox's. Why does it close the canvas element? Is it the /> at the end of the element? Is it the subsequent </div> or some other markup? Or what?
Simon Pride
Comment 6 2007-02-23 14:40:57 PST
Is it naive to ask why Safari 2.0.4 with its Tiger 10.4.8 version of WebKit (419.3?) handles it OK?
Darin Adler
Comment 7 2007-02-23 14:50:21 PST
(In reply to comment #6) > Is it naive to ask why Safari 2.0.4 with its Tiger 10.4.8 version of WebKit > (419.3?) handles it OK? In Tiger WebKit, <canvas> is a standalone element like <img>, and so there's no such things as an "open" canvas element. TOT WebKit has changed to match other browsers and the WhatWG draft specification, and thus you have to close the <canvas> element with a separate </canvas> in HTML documents. Firefox matches TOT WebKit in this regard, but somehow the site is still working. That's what we have to figure out.
Alexey Proskuryakov
Comment 8 2007-02-24 00:33:05 PST
Created attachment 13358 [details] test case > We should figure out why WebKit's rule doesn't match Firefox's. Why does it > close the canvas element? Is it the /> at the end of the element? Is it the > subsequent </div> or some other markup? Or what? Looks like it's a <div> </div> pair around the canvas - the closing div alone isn't enough.
Maciej Stachowiak
Comment 9 2007-02-27 19:34:15 PST
Darin Adler
Comment 10 2007-03-10 08:06:11 PST
I tested Firefox with other tags; title, style, iframe, noembed, noframes, noscript Only canvas had the special behavior, where the </div> closed the element.
Darin Adler
Comment 11 2007-03-10 08:07:31 PST
Also, I tested with multiple elements, and it only worked if the element closed was the one opened just before the canvas. I'm guessing this is a canvas-specific quirk in Firefox.
Darin Adler
Comment 12 2007-03-10 08:12:41 PST
A </p> works, as does </div>, but not </a>, </b>, or </form>. And the element doesn't have to be the closest enclosing one.
Darin Adler
Comment 13 2007-03-10 08:14:34 PST
My mistake. Closing any open element seems to work. My experiments with <a> and <b> were presumably in cases where those elements were closed already.
Darin Adler
Comment 14 2007-03-10 08:29:29 PST
Seems that the Gecko HTML parser classifies canvas as a "special". http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/nsElementTable.cpp And that file contains all sorts of rules for closing. I'm still not sure how to modify our parser to have similar behavior in this case.
Darin Adler
Comment 15 2007-03-10 10:10:03 PST
Created attachment 13577 [details] patch with change log and test case
mitz
Comment 16 2007-03-10 11:27:58 PST
Comment on attachment 13577 [details] patch with change log and test case This sentence is not quite right: "Changed the Document parameter to the constructor to instead of HTMLDocument".
Maciej Stachowiak
Comment 17 2007-03-12 17:47:43 PDT
Some comments I noticed before deciding to question the approach: 1) I'd like to see more tests for cases that should or should not close the canvas. 2) The correct spelling is "well-formed", not "wellformed" or "well formed". And, finally, <canvas> should not skip parsing its contents - it should just prevent them from rendering only. That appears to be what Firefox does. One special case to consider is a <script> inside <canvas>, Firefox appears to execute it: <canvas> <b>fallback</b> <script> alert('x'); </script> </canvas>
Maciej Stachowiak
Comment 18 2007-03-12 17:50:47 PDT
Comment on attachment 13577 [details] patch with change log and test case r-
Darin Adler
Comment 19 2007-03-13 08:58:15 PDT
Created attachment 13611 [details] revised patch, with change log and regression tests
Maciej Stachowiak
Comment 20 2007-03-13 16:08:24 PDT
Comment on attachment 13611 [details] revised patch, with change log and regression tests r=me
Darin Adler
Comment 21 2007-03-13 16:33:27 PDT
Note You need to log in before you can comment on or make changes to this bug.