[webkit-reviews] review denied: [Bug 26333] alert during a dragenter event handler will crash the renderer : [Attachment 31178] Porpsed Fix for Bug 26333

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Victor Wang <victorw at chromium.org>'s
request for review:
Bug 26333: alert during a dragenter event handler will crash the renderer
https://bugs.webkit.org/show_bug.cgi?id=26333

Attachment 31178: Porpsed Fix for Bug 26333
https://bugs.webkit.org/attachment.cgi?id=31178&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
You should set CHANGELOG_NAME in your environment.  or REAL_NAME. 
prepare-ChangeLog isn't smart enough on windows to be able to get your real
name automatically, hence:
 1 2009-06-11  victorw	<victorw at chromium.org>

You should mention why this needs to be a manual test (because DRT doesn't show
alerts)

Officially our style guidelines don't use { }  on single line ifs:
+  if (window.layoutTestController) {
+      layoutTestController.dumpAsText();
+  }
I'm not sure that our style guide is clear about that for JavaScript, so it
might not matter.

WK style is also to use more descriptive variable names:
+  var e = document.documentElement;
+  var f = function() {
+    alert('Click OK button.');
+  };
at least in code. ;)

Probably should say WebKit:
+    <p>Do the following and see if Chromium crashes.</p>

Because that will crash Safari too, no?

Otherwise looks fine.  r- for the nits above.