Bug 12794 – REGRESSION: TripTik planner at aaa.com never finishes loading due to unclosed canvas tag

• Component: DOM
• Status: RESOLVED FIXED

[Simon Pride]

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

[Alexey Proskuryakov]

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]

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]

Does this fail with Firefox too? I ask because I believe Firefox would also consider this an unclosed <canvas> tag.

[Simon Pride]

> 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]

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]

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]

(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]

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]

<rdar://problem/5028154>

[Darin Adler]

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]

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]

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]

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]

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]

Created attachment 13577 [details]
patch with change log and test case

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]

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 on attachment 13577 [details]
patch with change log and test case

r-

[Darin Adler]

Created attachment 13611 [details]
revised patch, with change log and regression tests

[Maciej Stachowiak]

Comment on attachment 13611 [details]
revised patch, with change log and regression tests

r=me

[Darin Adler]

Committed revision 20170. <http://trac.webkit.org/projects/webkit/changeset/20170>