Bug 218678 – Web Inspector: drop `shown`/`hidden` in favor of `attached`/`detached`

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

[Devin Rousso]

- `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

[Devin Rousso]

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

[Devin Rousso]

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.

[Devin Rousso]

Created attachment 413898 [details]
Patch

[Devin Rousso]

Created attachment 413899 [details]
Patch

[Blaze Burg]

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.

[Devin Rousso]

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

[Radar WebKit Bug Importer]

<rdar://problem/71395601>

[Blaze Burg]

Comment on attachment 413899 [details]
Patch

r=me

[EWS]

Committed r270134: <https://trac.webkit.org/changeset/270134>

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