[webkit-reviews] review granted: [Bug 180520] ServiceWorker Inspector: Various issues inspecting service worker on mobile.twitter.com : [Attachment 328678] [PATCH] Proposed Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 8 14:47:27 PST 2017


Brian Burg <bburg at apple.com> has granted Joseph Pecoraro <joepeck at webkit.org>'s
request for review:
Bug 180520: ServiceWorker Inspector: Various issues inspecting service worker
on mobile.twitter.com
https://bugs.webkit.org/show_bug.cgi?id=180520

Attachment 328678: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=328678&action=review




--- Comment #6 from Brian Burg <bburg at apple.com> ---
Comment on attachment 328678
  --> https://bugs.webkit.org/attachment.cgi?id=328678
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=328678&action=review

r=me

> Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:647
> +	   // Create a main resource with this content in case the content
never shows up as a WI.Script.

Please rename the containing method to _processServiceWorkerConfiguration since
you renamed the type in this patch. This comment may apply elsewhere.

> Source/WebInspectorUI/UserInterface/Controllers/FrameResourceManager.js:655
> +		   console.log("USING LOCAL RESOURCE");

Nit: Please remove

> Source/WebInspectorUI/UserInterface/Models/TextRange.js:59
> +	   return new WI.TextRange(0, 0, lines.length - 1,
lines.lastValue.length);

Is the end column not also zero-based? I would have expected
lines.lastValue.length - 1.


More information about the webkit-reviews mailing list