Bug 230286 – Web Inspector: Don't maintain a back-forward stack for `ContentBrowser`/`ContentViewContainer` when not necessary

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

[Patrick Angle]

Many content views explicitly request hidden back/forward navigation buttons, but many of them also do not need to maintain a full stack of back/forward history for navigation. This makes it more difficult for us to ensure that stale views are discarded in a timely manner, particularly on page reload.

[Radar WebKit Bug Importer]

<rdar://problem/83125792>

[Patrick Angle]

Created attachment 438202 [details]
Patch v1.0

[Devin Rousso]

Comment on attachment 438202 [details]
Patch v1.0

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

r=me, awesome stuff =D

> Source/WebInspectorUI/UserInterface/Views/AuditTabContentView.js:32
> +            disableBackForwardButtons: true,

NIT: `disableBackForwardButtons` makes me think that it's gonna leave the buttons where they are but just `.enabled = false`.  Perhaps `hideBackFowardButtons` instead?

> Source/WebInspectorUI/UserInterface/Views/AuditTabContentView.js:33
> +            disableBackForwardNavigation: true,

Does this tab also need to remember selection like the Graphics Tab, or am I misremembering/mistaken?

> Source/WebInspectorUI/UserInterface/Views/NetworkDetailView.js:88
> +        this._contentBrowser = new WI.ContentBrowser(element, this, {disableBackForwardButtons: true, disableBackForwardNavigation: true, contentViewNavigationItemsFlexItem, contentViewNavigationItemsGroup});

I feel like this may want to keep a back-forward list for the various alternate representations of the resource's content (e.g. "Response" vs "Response (JSON)").  Can you confirm?

[Patrick Angle]

Comment on attachment 438202 [details]
Patch v1.0

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

>> Source/WebInspectorUI/UserInterface/Views/AuditTabContentView.js:33
>> +            disableBackForwardNavigation: true,
> 
> Does this tab also need to remember selection like the Graphics Tab, or am I misremembering/mistaken?

I've confirmed you are correct - this was hidden before by Bug 230322 which hid either the only, or most noticeable, use for the stack in the Audit tab.

>> Source/WebInspectorUI/UserInterface/Views/NetworkDetailView.js:88
>> +        this._contentBrowser = new WI.ContentBrowser(element, this, {disableBackForwardButtons: true, disableBackForwardNavigation: true, contentViewNavigationItemsFlexItem, contentViewNavigationItemsGroup});
> 
> I feel like this may want to keep a back-forward list for the various alternate representations of the resource's content (e.g. "Response" vs "Response (JSON)").  Can you confirm?

This state isn't remembered in ToT/STP... I'll open a bug for that.

[Patrick Angle]

Comment on attachment 438202 [details]
Patch v1.0

This patch appears to have unintentionally moved the "Response" format dropdown menu in network details from being right justified to being left justified because of a name mismatch for that variable in this patch. contentViewNavigationItemsGroup is also affected.

[Patrick Angle]

Created attachment 438386 [details]
Patch v1.1

[Patrick Angle]

Created attachment 438387 [details]
Patch v1.1.1 - Add reviewer to change log

[EWS]

Committed r282610 (241770@main): <https://commits.webkit.org/241770@main>

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