Bug 218678

Summary: Web Inspector: drop `shown`/`hidden` in favor of `attached`/`detached`
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, eric.carlson, ews-watchlist, glenn, hi, inspector-bugzilla-changes, jer.noble, joepeck, philipj, sergio, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218836
Attachments:
Description Flags
[Patch] WIP
hi: commit-queue-
[Patch] WIP
none
Patch
none
Patch none

Description Devin Rousso 2020-11-06 20:53:51 PST 
- `attached`/`detached` is tied into `WI.View`, meaning that `attached` is called right after the node is added to the main DOM tree and `detached` is called right before the node is removed from the main DOM tree (we should be removing nodes that aren't visible from the DOM anyways)
 - `shown`/`hidden` are usually called right after `addSubview`/`removeSubview`, which should have already called `attached`/`detached`
 - `attached`/`detached` is already propagated to the entire view subtree, whereas `shown`/`hidden` must be propagated manually
Comment 1 Devin Rousso 2020-11-06 21:05:42 PST 
Created attachment 413514 [details]
[Patch] WIP

I think this all works, but I'm going to live on it for a bit before writing a ChangeLog and such
Comment 2 Devin Rousso 2020-11-11 17:43:33 PST 
Created attachment 413891 [details]
[Patch] WIP

Fixed an issue I just discovered where switching to the Console Tab when the split console is showing caused the split console to always appear blank.  This happens because the `WI.ConsoleDrawer` is never removed from the DOM even when hidden, meaning that `detached` is never called on it's `WI.ContentViewContainer` which is what's responsible for hiding all of the entries in the back/forward list and therefore when the `WI.consoleContentView` is re-shown (via `showContentView`) it matches the current item in the back/forward list and is not re-added.  Changed `WI.ConsoleDrawer.prototype.set collapsed` to manually call `attached`/`detached`.

Alongside this, I also added a fix/drive-by for re-showing the split console when switching away from the Console Tab if the split console was previously shown.
Comment 3 Devin Rousso 2020-11-11 18:48:44 PST 
Created attachment 413898 [details]
Patch
Comment 4 Devin Rousso 2020-11-11 18:52:22 PST 
Created attachment 413899 [details]
Patch
Comment 5 BJ Burg 2020-11-13 11:34:34 PST 
Comment on attachment 413899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413899&action=review

> Source/WebInspectorUI/UserInterface/Views/View.js:129
> +        view._didMoveToParent(null);

I feel it's more accurate that this would be _willMoveToParent() since it happens prior to element.remove()... but it probably doesn't matter.
Comment 6 Devin Rousso 2020-11-13 12:40:59 PST 
Comment on attachment 413899 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413899&action=review

>> Source/WebInspectorUI/UserInterface/Views/View.js:129
>> +        view._didMoveToParent(null);
> 
> I feel it's more accurate that this would be _willMoveToParent() since it happens prior to element.remove()... but it probably doesn't matter.

IMO it either should have a `will*`/`did*` pair or not have any tense at all (e.g. `moveToParent`).  But yeah it's just naming :P
Comment 7 Radar WebKit Bug Importer 2020-11-13 20:54:14 PST 
<rdar://problem/71395601>
Comment 8 BJ Burg 2020-11-20 14:19:42 PST 
Comment on attachment 413899 [details]
Patch

r=me
Comment 9 EWS 2020-11-20 14:37:07 PST 
Committed r270134: <https://trac.webkit.org/changeset/270134>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 413899 [details].