Bug 151643

Summary:

Web Inspector: show something useful when the inspector frontend fails to load

Product:

WebKit

Reporter:

Blaze Burg <bburg>

Component:

Web Inspector

Assignee:

Blaze Burg <bburg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
[Screenshot] Undocked Inspector	none
[Screenshot] Docked inspector	none
Proposed Fix	none
Proposed Fix	timothy: review+

Blaze Burg

Reported 2015-11-28 18:40:55 PST

This is a huge mystery even if you know something went wrong. We can easily provide: - An oops message - The actual errors - link to a prefilled bugzilla bug - A link that reloads the frontend

Attachments
[Screenshot] Undocked Inspector (423.32 KB, image/png) 2015-11-28 18:41 PST, Blaze Burg	no flags	Details
[Screenshot] Docked inspector (1.19 MB, image/png) 2015-11-28 18:41 PST, Blaze Burg	no flags	Details
Proposed Fix (9.96 KB, patch) 2015-11-28 22:41 PST, Blaze Burg	no flags	Details Formatted Diff Diff
Proposed Fix (10.50 KB, patch) 2015-11-28 23:36 PST, Blaze Burg	timothy: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2015-11-28 18:41:15 PST

<rdar://problem/23681343>

Blaze Burg

Comment 2 2015-11-28 18:41:33 PST

Created attachment 266217 [details] [Screenshot] Undocked Inspector

Blaze Burg

Comment 3 2015-11-28 18:41:58 PST

Created attachment 266218 [details] [Screenshot] Docked inspector

Blaze Burg

Comment 4 2015-11-28 22:41:39 PST

Created attachment 266220 [details] Proposed Fix

WebKit Commit Bot

Comment 5 2015-11-28 22:44:11 PST

Attachment 266220 [details] did not pass style-queue: ERROR: Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:63: Line contains single-quote character. [js/syntax] [5] ERROR: Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:64: Line contains single-quote character. [js/syntax] [5] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Blaze Burg

Comment 6 2015-11-28 23:36:18 PST

Created attachment 266221 [details] Proposed Fix New patch fixes some issues that come up if a ReferenceError happens after some of the inspector has been loaded, such as in WebInspector.contentLoaded. Unconditionally disarm all event handlers to prevent other weirdness.

WebKit Commit Bot

Comment 7 2015-11-28 23:37:24 PST

Attachment 266221 [details] did not pass style-queue: ERROR: Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:75: Line contains single-quote character. [js/syntax] [5] ERROR: Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:76: Line contains single-quote character. [js/syntax] [5] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Nikita Vasilyev

Comment 8 2015-11-29 19:21:08 PST

Comment on attachment 266221 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266221&action=review Nice! Text could be a tad smaller, especially in the docked state. I'd change margins and padding too. I think it would be easier if I send a follow-up patch. > Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.css:61 > +} Nit: no empty CSS rules in the patch ready for landing. > Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:32 > + document.write("<body></body></html>"); Do we have to use document.write? Can we do: if (!document.body) document.body = document.createElement("body"); ? > Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:44 > + document.addEventListener(name, stopEventPropagation, true); When would we need to do this? > Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:69 > + Web Inspector encountered an error while loading. No need for dot at the end of title. I'm not a reviewer, so I'll keep it r?.

Blaze Burg

Comment 9 2015-11-30 09:29:25 PST

Comment on attachment 266221 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266221&action=review >> Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:32 >> + document.write("<body></body></html>"); > > Do we have to use document.write? > Can we do: > > if (!document.body) > document.body = document.createElement("body"); > > ? I see no difference. >> Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:44 >> + document.addEventListener(name, stopEventPropagation, true); > > When would we need to do this? See the comment that was posted with the second patch. I added a shorter comment above. >> Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:69 >> + Web Inspector encountered an error while loading. > > No need for dot at the end of title. > > I'm not a reviewer, so I'll keep it r?. Everything else on the page is a full sentence.

Blaze Burg

Comment 10 2015-11-30 09:34:04 PST

(In reply to comment #8) > Comment on attachment 266221 [details] > Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266221&action=review > > Nice! > > Text could be a tad smaller, especially in the docked state. I'd change > margins and padding too. I think it would be easier if I send a follow-up > patch. I prefer bigger text, since the error page doesn't zoom. That said, it's a step bigger than what Reader uses by default. To match what Reader uses, I can reduce the font size a bit (but keep the Sans Serif).

Timothy Hatcher

Comment 11 2015-11-30 10:25:58 PST

Comment on attachment 266221 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266221&action=review >>> Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:32 >>> + document.write("<body></body></html>"); >> >> Do we have to use document.write? >> Can we do: >> >> if (!document.body) >> document.body = document.createElement("body"); >> >> ? > > I see no difference. We try not to use things like innerHTML, document.write unless it is needed.

Blaze Burg

Comment 12 2015-11-30 15:27:35 PST

Committed r192837: <http://trac.webkit.org/changeset/192837>

Joseph Pecoraro

Comment 13 2015-12-01 10:41:59 PST

I think this caused a broken inspector on minified builds. The Debug/Foo.js being early on may be causing the minified/combined Main.html to output Main.js before its dependencies: <script src="Main.js"></script> <script src="CodeMirror.js"></script> <script src="Esprima.js"></script> <script src="ESLint.js"></script>

Joseph Pecoraro

Comment 14 2015-12-01 10:45:28 PST

Comment on attachment 266221 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266221&action=review > Source/WebInspectorUI/UserInterface/Debug/CatchEarlyErrors.js:68 > + <img src="Images/Errors.svg" /> No need for "/>" on the known self-closing tags like <img>.

Note You need to log in before you can comment on or make changes to this bug.