Created attachment 305238[details]
[Image] new tree element titles
Summary:
XHR breakpoints should be global. They are currently associated with the main frame URL.
Note:
Tree element titles for XHR breakpoints should "URL — <url text>", instead of "URL contains: <url text>". See screenshot.
Created attachment 305307[details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 305297[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=305297&action=review> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
> + if (this._xhrBreakpoints.some((entry) => entry.url === breakpoint.url))
> + return;
How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.
> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:363
> + breakpoint.dispatchEventToListeners(WebInspector.XHRBreakpoint.Event.XHRBreakpointResolved);
My checkout doesn't have this event name. What is this? r- for this
> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css:112
> .sidebar > .panel.navigation.debugger .details-section.xhr-breakpoints .item.breakpoint .subtitle {
> - padding-left: 5px;
> font-family: Menlo, monospace;
Food for thought: If we supported regular expressions (which I think we should because it would be a very small patch) we could use a FormattedValue here. We could show:
URL – "foo"
URL – /foo\d/
or:
URL contains: "foo"
URL matches: /foo\d/
And they would be syntax highlighted like they are in the console. What you have now looks good to me, far more compact.
Comment on attachment 305297[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=305297&action=review>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
>> + return;
>
> How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.
The ".some(() => ...)" test prevents this from happening. No message is shown to the user when attempting to add an XHR breakpoint with a duplicate.
>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:363
>> + breakpoint.dispatchEventToListeners(WebInspector.XHRBreakpoint.Event.XHRBreakpointResolved);
>
> My checkout doesn't have this event name. What is this? r- for this
This is a typo. Fixed.
>> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.css:112
>> font-family: Menlo, monospace;
>
> Food for thought: If we supported regular expressions (which I think we should because it would be a very small patch) we could use a FormattedValue here. We could show:
>
> URL – "foo"
> URL – /foo\d/
>
> or:
>
> URL contains: "foo"
> URL matches: /foo\d/
>
> And they would be syntax highlighted like they are in the console. What you have now looks good to me, far more compact.
This is exactly what I'd like to do! We should also use a CodeMirror instance in InputPopover to take advantage of CodeMirror's regex parsing/highlighting. Filed a follow-up:
Web Inspector: Add regular expression support to XHR breakpoints
https://bugs.webkit.org/show_bug.cgi?id=170099
Created attachment 305383[details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 305384[details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 305385[details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 305390[details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 305391[details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 305392[details]
Archive of layout-test-results from ews116 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 305413[details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 305464[details]
Archive of layout-test-results from ews122 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 305297[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=305297&action=review>>> Source/WebInspectorUI/UserInterface/Controllers/DOMDebuggerManager.js:186
>>> + return;
>>
>> How does the UI handle this? What does the UI display when the user adds the same URL breakpoint twice? It looks like the UI would show it twice, until we navigate and then it would show 1. That might need to be improved.
>
> The ".some(() => ...)" test prevents this from happening. No message is shown to the user when attempting to add an XHR breakpoint with a duplicate.
Oh, so the user attempts to add a breakpoint but no new breakpoint shows up because one already exists with the same URL. Okay, that seems fine.
2017-03-23 16:26 PDT, Matt Baker
2017-03-24 10:54 PDT, Matt Baker
2017-03-24 11:41 PDT, Build Bot
2017-03-25 09:44 PDT, Matt Baker
2017-03-25 10:51 PDT, Build Bot
2017-03-25 11:00 PDT, Build Bot
2017-03-25 11:05 PDT, Build Bot
2017-03-25 11:06 PDT, Matt Baker
2017-03-25 12:13 PDT, Build Bot
2017-03-25 12:18 PDT, Build Bot
2017-03-25 12:29 PDT, Build Bot
2017-03-25 13:42 PDT, Matt Baker
2017-03-25 20:58 PDT, Build Bot
2017-03-26 08:21 PDT, Matt Baker
2017-03-27 06:29 PDT, Build Bot
2017-03-30 13:03 PDT, Matt Baker