Summary: Show pause reason for "All Requests" XHR breakpoint. Currently the frontend skips updating the UI when pauseData.breakpointURL === "".
<rdar://problem/31649963>
Created attachment 307246 [details] Patch
Created attachment 307247 [details] Patch
Created attachment 307248 [details] [Image] Debugger sidebar (before)
Created attachment 307249 [details] [Image] Debugger sidebar (after)
Comment on attachment 307247 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307247&action=review r=me > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1052 > + } else { console.assert(pauseData.breakpointURL === "", "Should be the All Request breakpoint which has an empty URL"); > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1053 > + this._pauseReasonTextRow.text = WebInspector.UIString("XHR sent: %s").format(pauseData.url); I don't think "XHR sent" is correct. We haven't yet sent the XHR, we are about to send it. As we saw last week, you should be able to change this XHR before it is actually sent. Some ideas: "Sending: %s" "Requesting: %s" "Sending Request: %s" "URL: %s"
(In reply to Joseph Pecoraro from comment #6) > Comment on attachment 307247 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307247&action=review > > r=me > > > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1052 > > + } else { > > console.assert(pauseData.breakpointURL === "", "Should be the All Request > breakpoint which has an empty URL"); > > > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1053 > > + this._pauseReasonTextRow.text = WebInspector.UIString("XHR sent: %s").format(pauseData.url); > > I don't think "XHR sent" is correct. We haven't yet sent the XHR, we are > about to send it. As we saw last week, you should be able to change this XHR > before it is actually sent. > > Some ideas: > > "Sending: %s" > "Requesting: %s" > "Sending Request: %s" > "URL: %s" These are all good suggestions. I choose "Requesting", since it's the verb used to label the "All Requests" breakpoint.
Created attachment 307288 [details] Patch
Comment on attachment 307288 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307288&action=review > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1057 > + this._pauseReasonTextRow.element.title = pauseData.url; Is there any reason to set the tooltip when it matches the current text?
(In reply to Joseph Pecoraro from comment #9) > Comment on attachment 307288 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307288&action=review > > > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:1057 > > + this._pauseReasonTextRow.element.title = pauseData.url; > > Is there any reason to set the tooltip when it matches the current text? Long URLs can be truncated.
The Pause Reason section could do a better job of displaying long URLs/exception text.
Comment on attachment 307288 [details] Patch Clearing flags on attachment: 307288 Committed r215427: <http://trac.webkit.org/changeset/215427>
All reviewed patches have been landed. Closing bug.