RESOLVED FIXED201717
Web Inspector: Improve the Uncaught Exception View file a bug link
https://bugs.webkit.org/show_bug.cgi?id=201717
Summary Web Inspector: Improve the Uncaught Exception View file a bug link
Joseph Pecoraro
Reported 2019-09-12 01:30:38 PDT
Improve the Uncaught Exception View file a bug link * Link doesn't work today * Update the pre-filled comment style * Bring the new tab to the foreground if possible
Attachments
[PATCH] Proposed Fix (8.55 KB, patch)
2019-09-12 01:35 PDT, Joseph Pecoraro
hi: review+
[IMAGE] After - Uncaught Exception Reporter (656.64 KB, image/png)
2019-09-12 01:36 PDT, Joseph Pecoraro
no flags
[IMAGE] After - New Bug Report (616.16 KB, image/png)
2019-09-12 01:36 PDT, Joseph Pecoraro
no flags
Joseph Pecoraro
Comment 1 2019-09-12 01:35:55 PDT
Created attachment 378632 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 2 2019-09-12 01:36:17 PDT
Created attachment 378633 [details] [IMAGE] After - Uncaught Exception Reporter
Joseph Pecoraro
Comment 3 2019-09-12 01:36:42 PDT
Created attachment 378634 [details] [IMAGE] After - New Bug Report
Devin Rousso
Comment 4 2019-09-20 19:03:24 PDT
Comment on attachment 378632 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378632&action=review r=me One thought, should we expose this (or something like this) for STP? We may find out about more bugs that way 🤔 > Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js:-283 > - bug with this information</a>. It is possible that someone else broke it by accident.</dd> lol I didn’t know it said this 😂 > Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js:301 > + event.stop(); Should we avoid using Utilities.js functions in this file, in case it wasn’t loaded properly and therefore the extra functions don’t exist? > Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:270 > + WebProcess::singleton().parentProcessConnection()->send(Messages::WebInspectorProxy::BringInspectedPageToFront(), m_inspectedPageIdentifier); Shouldn’t we bring to front before opening a new tab? Just thinking about it, I’d expect the inspected page/tab/window to be focused before attempting to open another tab. Alternatively, should we just always bring the inspected page/tab/window to front in the UIProcess whenever calling `openInNewTab`? Is there a situation where we’d want to open a background tab?
Joseph Pecoraro
Comment 5 2019-09-23 10:57:25 PDT
Comment on attachment 378632 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378632&action=review >> Source/WebInspectorUI/UserInterface/Debug/UncaughtExceptionReporter.js:301 >> + event.stop(); > > Should we avoid using Utilities.js functions in this file, in case it wasn’t loaded properly and therefore the extra functions don’t exist? Good idea. I'll expand this. >> Source/WebKit/WebProcess/WebPage/WebInspectorUI.cpp:270 >> + WebProcess::singleton().parentProcessConnection()->send(Messages::WebInspectorProxy::BringInspectedPageToFront(), m_inspectedPageIdentifier); > > Shouldn’t we bring to front before opening a new tab? Just thinking about it, I’d expect the inspected page/tab/window to be focused before attempting to open another tab. > > Alternatively, should we just always bring the inspected page/tab/window to front in the UIProcess whenever calling `openInNewTab`? Is there a situation where we’d want to open a background tab? Hmm, maybe we should do something like Safari's preferences. ⌘-click versus ⌘-Shift-click. Where Shift activates the new tab. That said I think activating is a clear improvement for now. Unlike in Safari where you might like to open a lot of links from an article, opening content from Web Inspector generally is an operation where you want to go to the content you just opened.
Joseph Pecoraro
Comment 6 2019-09-23 11:07:10 PDT
Radar WebKit Bug Importer
Comment 7 2019-09-23 11:08:25 PDT
Note You need to log in before you can comment on or make changes to this bug.