|Summary:||Web Inspector: subclasses of WI.ClusterContentView don't save/restore content views after the initial view|
|Product:||WebKit||Reporter:||Devin Rousso <hi>|
|Component:||Web Inspector||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, simon.fraser, webkit-bug-importer|
|Version:||WebKit Nightly Build|
Description Devin Rousso 2018-12-05 20:37:53 PST
Comment 1 Devin Rousso 2018-12-05 20:38:03 PST
*** Bug 190967 has been marked as a duplicate of this bug. ***
Comment 2 Devin Rousso 2018-12-05 20:38:08 PST
*** Bug 192445 has been marked as a duplicate of this bug. ***
Comment 3 Devin Rousso 2018-12-05 21:10:50 PST
I think the reason this is happening is because each `WI.ClusterContentView` (C) has a `WI.ContentViewContainer`, which contains a `_backForwardList` (C list). The `WI.ClusterContentView` itself is contained within a `WI.ContentViewContainer` (P), meaning that it's parent also has a `_backForwardList` (P list). When we change views within C, we add a `WI.BackForwardEntry` to C list, but NOT to P list, meaning that when we re-show P after switching tabs, which will contain only 1 `WI.BackForwardEntry` in P list, which was for the original first time that P was shown. tl;dr: Switching views within the cluster will add history entires to the cluster, but not to it's parent, meaning that the parent will be unaware of how it's child has changed and will default to showing what it knows, which is what the child originally showed.
Comment 4 Devin Rousso 2018-12-06 09:20:53 PST
This is compounded by the fact that when P calls `shown()`, we use the initial `WI.BackForwardEntry` (I) of P to insert a new `WI.BackForwardEntry` (N) of C, meaning that when C calls `shown()` it sees N instead of I and displays that.
Comment 5 Radar WebKit Bug Importer 2018-12-17 21:33:35 PST
Comment 6 Joseph Pecoraro 2019-01-04 13:42:16 PST
I think ClusterContentViews will probably want to save their state when they hide, otherwise they will be restored incorrectly. I can try a fix like that.
Comment 7 Devin Rousso 2019-01-04 14:03:55 PST
(In reply to Joseph Pecoraro from comment #6) > I think ClusterContentViews will probably want to save their state when they hide, otherwise they will be restored incorrectly. I can try a fix like that. The issue I think is more the idea that changes to the `WI.ClusterContentView` (C) are only saved by C itself, not any of its parents. In the example I gave (comment #3), the main problem was that changes to C didn't propagate to P, which meant that when P was show (P is the "first responder"), it would use the last item in it's list, which was for the original showing of C. One possible thing we could do is have all `WI.ClusterContentView` somehow propagate their changes to their parent (if any) so that the next time the parent is shown, it has the new state.
Comment 8 Joseph Pecoraro 2019-01-04 14:28:50 PST
Created attachment 358372 [details] [PATCH] Proposed Fix Tested this with: - JS Timeline - Call Trees / Events - Toggle between tabs after toggling this picker - Heap Timelines - Instances / Object Graph - Toggle between tabs after toggling this picker - SVG Image view - Image / Source - Toggle between an SVG and PNG after toggling this picker The back/forward list felt a little messed up in Timelines but it already feels a little confused, so it is no worse. This however is much better default behavior for switching tabs with Timelines.
Comment 9 Devin Rousso 2019-01-04 15:30:56 PST
Comment 10 Joseph Pecoraro 2019-01-04 16:07:09 PST
Created attachment 358390 [details] [PATCH] For Landing
Comment 11 WebKit Commit Bot 2019-01-04 17:24:00 PST
Comment on attachment 358390 [details] [PATCH] For Landing Clearing flags on attachment: 358390 Committed r239649: <https://trac.webkit.org/changeset/239649>
Comment 12 EWS Watchlist 2019-01-04 17:29:58 PST
Comment on attachment 358372 [details] [PATCH] Proposed Fix Attachment 358372 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10634375 New failing tests: http/wpt/css/css-animations/start-animation-001.html
Comment 13 EWS Watchlist 2019-01-04 17:29:59 PST
Created attachment 358404 [details] Archive of layout-test-results from ews114 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-sierra Platform: Mac OS X 10.12.6