[webkit-reviews] review granted: [Bug 33864] REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset() : [Attachment 46951] Patch

Tue Jan 19 14:10:13 PST 2010

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Enrica Casucci
<enrica at apple.com>'s request for review:
Bug 33864: REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset()
https://bugs.webkit.org/show_bug.cgi?id=33864

Attachment 46951: Patch
https://bugs.webkit.org/attachment.cgi?id=46951&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================

> +	   REGRESSION: Infinite recursion in Position::getInlineBoxAndOffset()
> +	   https://bugs.webkit.org/show_bug.cgi?id=33864
> +	   <rdar://problem/7552959>
> +	   
> +	   Regression has been introduced by changeset 53085.
> +	   The original fix had to be limited to editable content.

Could you reword this to explain it without reference to the original change?

> Index: LayoutTests/editing/selection/selection-applet.html
> ===================================================================
> --- LayoutTests/editing/selection/selection-applet.html	(revision 0)
> +++ LayoutTests/editing/selection/selection-applet.html	(revision 0)
> @@ -0,0 +1,10 @@
> +<body>
> +<script language="JavaScript" type="text/javascript">
> +if (window.layoutTestController)
> +	layoutTestController.dumpAsText();
> +
> +document.write('<applet id="test" width="400" height="400" name="test
applet" code="test.class" codebase="." mayscript>');
> +document.write('You need Java(tm) to view this presentation. <a
href="http://www.java.com">Download Java</a></applet> ');
> +window.getSelection().setPosition(document.getElementById("test"), 0);
> +</script>
> +</body>

Does the testcase really require Java, or would a simpler test with divs
suffice? Also, is it required to document.write the test content?

r=me, but it would be nice to make the testcase more generic if possible.