RESOLVED FIXED151643
Web Inspector: show something useful when the inspector frontend fails to load 
https://bugs.webkit.org/show_bug.cgi?id=151643
Summary Web Inspector: show something useful when the inspector frontend fails to load
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
DetailsFormatted DiffDiff
Proposed Fix (10.50 KB, patch)
2015-11-28 23:36 PST, Blaze Burg 		timothy: review+
DetailsFormatted DiffDiff
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.