Bug 170895 – Web Inspector: Show pause reason for "All Requests" XHR breakpoint

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

Summary:
Show pause reason for "All Requests" XHR breakpoint. Currently the frontend skips updating the UI when pauseData.breakpointURL === "".

[Radar WebKit Bug Importer]

<rdar://problem/31649963>

[Matt Baker]

Created attachment 307246 [details]
Patch

[Matt Baker]

Created attachment 307247 [details]
Patch

[Matt Baker]

Created attachment 307248 [details]
[Image] Debugger sidebar (before)

[Matt Baker]

Created attachment 307249 [details]
[Image] Debugger sidebar (after)

[Joseph Pecoraro]

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"

[Matt Baker]

(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.

[Matt Baker]

Created attachment 307288 [details]
Patch

[Joseph Pecoraro]

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?

[Matt Baker]

(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.

[Matt Baker]

The Pause Reason section could do a better job of displaying long URLs/exception text.

[WebKit Commit Bot]

Comment on attachment 307288 [details]
Patch

Clearing flags on attachment: 307288

Committed r215427: <http://trac.webkit.org/changeset/215427>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.