[webkit-reviews] review denied: [Bug 25475] Show the filename and first line for "(program)" in the Profiler/Debugger : [Attachment 30924] patch with changes described in the bug up through 6/3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 16 11:59:14 PDT 2009


Timothy Hatcher <timothy at hatcher.name> has denied Patrick Mueller
<pmuellr at yahoo.com>'s request for review:
Bug 25475: Show the filename and first line for "(program)" in the
Profiler/Debugger
https://bugs.webkit.org/show_bug.cgi?id=25475

Attachment 30924: patch with changes described in the bug up through 6/3
https://bugs.webkit.org/attachment.cgi?id=30924&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +    // if no URL, look for "//@ sourceURL=" decorator     

This comment should mention that this matches what Firebug looks for and not
somthing we made up. Also remove the trailing whitespace.

> +	   // use of [ \t] rather than /s is to prevent \n from matching

This comment references /s which whould be \s.

> +	   if (match) {
> +	       this.sourceURL = "(eval):" + match[1];
> +	   }

No need for the braces around the if body. Remove them per our style guidlines.


Also I think this should say "(program): ". We don't use "eval" since it can be
code from setTimeout strings, new Function() or eval.

This should also be a localized string, and use WebInspector.UIString().

So WebInspector.UIString("(program): %s", match[1]).

Then add "(program): %s" to WebCore/English.lproj/localizedStrings.js.


More information about the webkit-reviews mailing list