Summary: Network tab content view is blank after reload. Only occurs when navigation sidebar is collapsed. Steps to Reproduce: 1. Inspect any page > Network tab 2. Collapse the left sidebar 3. Reload page => Content view is blank!
I was able to reproduce it.
<rdar://problem/30433456>
I was unable to reproduce this on ToT.
I still see this in ToT. I killed Safari local storage prior to testing just in case. Make sure the left sidebar is collapsed before reloading the page.
Created attachment 302429 [details] Patch
Comment on attachment 302429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302429&action=review Usually we require each change in WebKit to include a regression test. You can see some examples in LayoutTests/inspector. Your patch should include a regression test or, in those rare cases, an explanation of why no test is possible in this case. > Source/WebInspectorUI/ChangeLog:9 > + (WebInspector.NetworkSidebarPanel.prototype._networkTimelineReset): This is a good place to explain your understanding of the bug, and why your change makes things better. > Source/WebInspectorUI/ChangeLog:19 > +2017-02-22 Sam Brodkin <isam@apple.com> > + > + Web Inspector: Network tab content view is blank after reload > + https://bugs.webkit.org/show_bug.cgi?id=168029 > + > + Reviewed by NOBODY (OOPS!). > + > + * UserInterface/Views/NetworkSidebarPanel.js: > + (WebInspector.NetworkSidebarPanel.prototype._networkTimelineReset): Looks like prepare-ChangeLog ran twice. Better to remove the second entry.
Comment on attachment 302429 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302429&action=review > Source/WebInspectorUI/UserInterface/Views/NetworkSidebarPanel.js:194 > + this.showDefaultContentView(); In general we try to avoid doing work in tabs that aren't visible. A fairly common idiom used in the Inspector UI is: 1. Handle a message 2. is the view processing the message visible? 3. If so, update the view (on the next animation frame, if coalescing multiple events in quick succession) 4. If not, mark the view as having pending changes and update when it is shown For an example of this, see the treatment of "pending records" in Timeline content views.
(In reply to comment #6) > Comment on attachment 302429 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=302429&action=review > > Usually we require each change in WebKit to include a regression test. You > can see some examples in LayoutTests/inspector. Your patch should include a > regression test or, in those rare cases, an explanation of why no test is > possible in this case. Changes to the Inspector view hierarchy are not readily testable, although there has been some exploratory work done in this area. We typically require tests only for changes that impact the protocol, model hierarchy, or controllers/managers.
Created attachment 302439 [details] Patch
Comment on attachment 302439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302439&action=review > Source/WebInspectorUI/ChangeLog:11 > + showDefaultContentView() which shows networkGridView in the contentView was not being called when the NetworkSidebarPanel was not visible. Removing the if statement always shows the networkGridView which fixes the bug. This wouldn't be easy to regression test. Nit: We typically hard wrap changeling entries at about the column where 'called' is.
(In reply to comment #10) > Comment on attachment 302439 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=302439&action=review > > > Source/WebInspectorUI/ChangeLog:11 > > + showDefaultContentView() which shows networkGridView in the contentView was not being called when the NetworkSidebarPanel was not visible. Removing the if statement always shows the networkGridView which fixes the bug. This wouldn't be easy to regression test. > > Nit: We typically hard wrap changeling entries at about the column where > 'called' is. Changelog, lol.
Comment on attachment 302439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302439&action=review > Source/WebInspectorUI/UserInterface/Views/NetworkSidebarPanel.js:194 > + this.showDefaultContentView(); Have you tested this patch? I suspect that when the Network tab is in the background there will be layout assertions in NetworkGridContentView, related to laying out the graph column.
In order to test this, enable inspection of the Inspector window, so that you can monitor assertions: defaults write com.apple.Safari WebKitDeveloperExtrasEnabled -bool YES defaults write com.apple.Safari __WebInspectorPageGroupLevel1__.WebKit2DeveloperExtrasEnabled -bool YES
(In reply to comment #13) > In order to test this, enable inspection of the Inspector window, so that > you can monitor assertions: > > defaults write com.apple.Safari WebKitDeveloperExtrasEnabled -bool YES > defaults write com.apple.Safari > __WebInspectorPageGroupLevel1__.WebKit2DeveloperExtrasEnabled -bool YES That is so meta. I was wondering how to do this!
(In reply to comment #13) I inspected the Inspector window and saw no assertion failures (using an assertion failure breakpoint, looking in the console, and in the Activity Viewer) when reloading the network tab and changing to another tab while it was in flight.
Created attachment 302682 [details] Patch
Comment on attachment 302682 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302682&action=review > Source/WebInspectorUI/ChangeLog:23 > + networkGridView which fixes the bug. This wouldn't be easy to regression test. Looks like prepare-ChangeLog ran twice, again. You should remove the section that has no comments.
(In reply to comment #17) I'm trying but every time I run webkit-patch upload it creates a new entry that I can't figure out how to edit
Comment on attachment 302682 [details] Patch Attachment 302682 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3188262 New failing tests: media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-drag.html media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-drag-is-prevented-over-button.html
Created attachment 302721 [details] Archive of layout-test-results from ews113 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 302682 [details] Patch r=me, I'll fix the changelog.
Created attachment 305759 [details] For Landing
Comment on attachment 305759 [details] For Landing Clearing flags on attachment: 305759 Committed r214551: <http://trac.webkit.org/changeset/214551>
All reviewed patches have been landed. Closing bug.