[Webkit-unassigned] [Bug 31721] Using jQuery to show/hide IMG elements crashes WebKit

Fri May 20 21:06:56 PDT 2011

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

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #94308|commit-queue?               |commit-queue-
               Flag|                            |

--- Comment #30 from Ryosuke Niwa <rniwa at webkit.org>  2011-05-20 21:06:55 PST ---
(From update of attachment 94308)
View in context: https://bugs.webkit.org/attachment.cgi?id=94308&action=review

I'm sorry but you'd have to polish up your test.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:1
> +<html>

Missing <!DOCTYPE html>

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:12
> +function doclick() {

camelCase.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:13
> +

Why does the every function start with a blank line?

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:23
> +        // it is pretty tricky to deal with accelerators in a decent way, but I suppose
> +        // just removing the underscores is a good enough way
> +

I don't think this comment adds any useful information.  Please remove it.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:32
> +    // As long as didn't crash, we passed.

This comment is redundant.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:38
> +function hidediv() {

camelCase.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:41
> +    var div = document.getElementById ("DIV");
> +    div.style.display="none";

No need for a temporary div.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:56
> +        if (!window.layoutTestController)
> +            return;
> +

Wrong indentation level.

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:63
> +        items = eventSender.contextClick();

What's the point in calling contextClick() here?

> LayoutTests/editing/pasteboard/copy-standalone-image-crash.html:67
> +        setTimeout(doclick, 10);

What's the point of this setTimeout?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.