[webkit-reviews] review granted: [Bug 122181] Please add a way to quick peek at failing tests at build.webkit.org/dashboard : [Attachment 220214] proposed patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has granted Alexey Proskuryakov
<ap at webkit.org>'s request for review:
Bug 122181: Please add a way to quick peek at failing tests at
build.webkit.org/dashboard
https://bugs.webkit.org/show_bug.cgi?id=122181

Attachment 220214: proposed patch
https://bugs.webkit.org/attachment.cgi?id=220214&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=220214&action=review


> Tools/ChangeLog:47
> +	   *
BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Popover
.js:

We should merge these back to the Web Inspector too, if they are applicable.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/U
tilities.js:73
> +function loadJSONP(url, jsonCallbackName, callback)

Another option would be to tweak JSON.load to support an optional
jsonCallbackName. Then just preprocess the result string (stripping the
callback) before doing JSON.parse. That seems cleaner than creating an
invisible iframe. And less likely to have XSS issues.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/U
tilities.js:77
> +    console.assert(typeof jsonCallbackName == "string");

I like to use === as much as possible.