Skip to content

Commit f01b6d1

Browse files
rcaliman-applercaliman-apple
committed
Web Inspector: Network: Request / Response menu ignores user’s previous setting
https://bugs.webkit.org/show_bug.cgi?id=308310 rdar://108231795 Reviewed by BJ Burg. The existing logic in `WI.ResourceClusterContentView` to reuse the last chosen content view for a resource has a timing issue. If it's a custom request content view, those views are created asynchronously after getting the resource content and determining if they're applicable. By this time, the default content view, Request, is used as a fallback on the first call to `WI.ResourceClusterContentView.prototype._showContentViewForIdentifier()`. Any last known saved preference per-resource is overwritten with the fallback. This is fixed by first waiting for the custom content views to load. However, users have requested remembering content view preferences per resource type instead of per single resource. This patch introduces a persistent mapping by MIME type to the last used content view type identifer to restore a user's prefered content view. * Source/WebInspectorUI/UserInterface/Base/Setting.js: * Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js: (WI.ResourceClusterContentView.prototype.attached): (WI.ResourceClusterContentView.prototype.restoreFromCookie): (WI.ResourceClusterContentView.prototype.showRequest): (WI.ResourceClusterContentView.prototype.showResponse): (WI.ResourceClusterContentView.prototype._showContentViewForIdentifier): (WI.ResourceClusterContentView.prototype._pathComponentSelected): (WI.ResourceClusterContentView.prototype._resourceLoadingDidFinish): (WI.ResourceClusterContentView.prototype._getMIMETypeForPreferences): (WI.ResourceClusterContentView.prototype._getPreferredContentViewIdentifier): (WI.ResourceClusterContentView.prototype._setPreferredContentViewIdentifier): (WI.ResourceClusterContentView.prototype._tryEnableCustomResponseContentViews): (WI.ResourceClusterContentView): Canonical link: https://commits.webkit.org/308142@main
1 parent b228c1b commit f01b6d1

File tree

2 files changed

+76
-35
lines changed

2 files changed

+76
-35
lines changed

‎Source/WebInspectorUI/UserInterface/Base/Setting.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ WI.settings = {
216216
searchFromSelection: new WI.Setting("search-from-selection", false),
217217
searchRegularExpression: new WI.Setting("search-regular-expression", false),
218218
selectedNetworkDetailContentViewIdentifier: new WI.Setting("network-detail-content-view-identifier", "preview"),
219+
resourceContentViewIdentifierForMIMEType: new WI.Setting("resource-content-view-identifier-for-mime-type", {}),
219220
sourceMapsEnabled: new WI.Setting("source-maps-enabled", true),
220221
showConsoleMessageTimestamps: new WI.Setting("show-console-message-timestamps", false),
221222
showCSSPropertySyntaxInDocumentationPopover: new WI.Setting("show-css-property-syntax-in-documentation-popover", false),

‎Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js‎

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,7 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
5151
this._tryEnableCustomRequestContentViews();
5252
}
5353

54-
// FIXME: Since a custom response content view may only become available after a response is received
55-
// we need to figure out a way to restore / prefer the custom content view. For example if users
56-
// always want to prefer the JSON view to the normal Response text view.
57-
58-
this._currentContentViewSetting = new WI.Setting("resource-current-view-" + this._resource.url.hash, ResourceClusterContentView.Identifier.Response);
59-
60-
this._tryEnableCustomResponseContentViews();
54+
this._enableCustomResponseContentViewsPromise = this._tryEnableCustomResponseContentViews();
6155
}
6256

6357
// Public
@@ -85,7 +79,14 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
8579
if (this._shownInitialContent)
8680
return;
8781

88-
this._showContentViewForIdentifier(this._currentContentViewSetting.value);
82+
this._enableCustomResponseContentViewsPromise
83+
.then(() => {
84+
let identifier = this._getPreferredContentViewIdentifier();
85+
this._showContentViewForIdentifier(identifier);
86+
})
87+
.catch((error) => {
88+
WI.reportInternalError(error);
89+
});
8990
}
9091

9192
closed()
@@ -97,39 +98,39 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
9798

9899
restoreFromCookie(cookie)
99100
{
100-
let contentView = this._showContentViewForIdentifier(cookie[WI.ResourceClusterContentView.ContentViewIdentifierCookieKey]);
101+
let contentViewIdentifier = cookie[WI.ResourceClusterContentView.ContentViewIdentifierCookieKey] || this._getPreferredContentViewIdentifier();
101102

102-
if (contentView.revealPosition) {
103-
let textRangeToSelect = null;
104-
if (!isNaN(cookie.startLine) && !isNaN(cookie.startColumn) && !isNaN(cookie.endLine) && !isNaN(cookie.endColumn))
105-
textRangeToSelect = new WI.TextRange(cookie.startLine, cookie.startColumn, cookie.endLine, cookie.endColumn);
103+
this._enableCustomResponseContentViewsPromise.then(() => {
104+
let contentView = this._showContentViewForIdentifier(contentViewIdentifier);
106105

107-
let position = null;
108-
if (!isNaN(cookie.lineNumber) && !isNaN(cookie.columnNumber))
109-
position = new WI.SourceCodePosition(cookie.lineNumber, cookie.columnNumber);
110-
else if (textRangeToSelect)
111-
position = textRangeToSelect.startPosition();
106+
if (contentView.revealPosition) {
107+
let textRangeToSelect = null;
108+
if (!isNaN(cookie.startLine) && !isNaN(cookie.startColumn) && !isNaN(cookie.endLine) && !isNaN(cookie.endColumn))
109+
textRangeToSelect = new WI.TextRange(cookie.startLine, cookie.startColumn, cookie.endLine, cookie.endColumn);
112110

113-
let scrollOffset = null;
114-
if (!isNaN(cookie.scrollOffsetX) && !isNaN(cookie.scrollOffsetY))
115-
scrollOffset = new WI.Point(cookie.scrollOffsetX, cookie.scrollOffsetY);
111+
let position = null;
112+
if (!isNaN(cookie.lineNumber) && !isNaN(cookie.columnNumber))
113+
position = new WI.SourceCodePosition(cookie.lineNumber, cookie.columnNumber);
114+
else if (textRangeToSelect)
115+
position = textRangeToSelect.startPosition();
116116

117-
if (position)
118-
contentView.revealPosition(position, {...cookie, textRangeToSelect, scrollOffset});
119-
}
117+
let scrollOffset = null;
118+
if (!isNaN(cookie.scrollOffsetX) && !isNaN(cookie.scrollOffsetY))
119+
scrollOffset = new WI.Point(cookie.scrollOffsetX, cookie.scrollOffsetY);
120+
121+
if (position)
122+
contentView.revealPosition(position, {...cookie, textRangeToSelect, scrollOffset});
123+
}
124+
});
120125
}
121126

122127
showRequest()
123128
{
124-
this._shownInitialContent = true;
125-
126129
return this._showContentViewForIdentifier(ResourceClusterContentView.Identifier.Request);
127130
}
128131

129132
showResponse()
130133
{
131-
this._shownInitialContent = true;
132-
133134
return this._showContentViewForIdentifier(ResourceClusterContentView.Identifier.Response);
134135
}
135136

@@ -337,7 +338,7 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
337338
return null;
338339
}
339340

340-
_showContentViewForIdentifier(identifier)
341+
_showContentViewForIdentifier(identifier, saveAsPreference)
341342
{
342343
let contentViewToShow = null;
343344

@@ -381,14 +382,18 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
381382

382383
console.assert(contentViewToShow);
383384

384-
this._currentContentViewSetting.value = this._identifierForContentView(contentViewToShow);
385+
if (saveAsPreference)
386+
this._setPreferredContentViewIdentifier(identifier);
387+
388+
this._shownInitialContent = true;
385389

386390
return this.contentViewContainer.showContentView(contentViewToShow);
387391
}
388392

389393
_pathComponentSelected(event)
390394
{
391-
this._showContentViewForIdentifier(event.data.pathComponent.representedObject);
395+
const saveAsPreference = true;
396+
this._showContentViewForIdentifier(event.data.pathComponent.representedObject, saveAsPreference);
392397
}
393398

394399
_resourceTypeDidChange(event)
@@ -408,7 +413,7 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
408413

409414
_resourceLoadingDidFinish(event)
410415
{
411-
this._tryEnableCustomResponseContentViews();
416+
this._enableCustomResponseContentViewsPromise = this._tryEnableCustomResponseContentViews();
412417
}
413418

414419
_canUseJSONContentViewForContent(content)
@@ -463,6 +468,41 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
463468
return mimeType;
464469
}
465470

471+
_getMIMETypeForPreferences()
472+
{
473+
if (!this._resource.mimeType)
474+
return null;
475+
476+
return parseMIMEType(this._resource.mimeType).type;
477+
}
478+
479+
_getPreferredContentViewIdentifier()
480+
{
481+
let mimeType = this._getMIMETypeForPreferences();
482+
if (!mimeType)
483+
return null;
484+
485+
let preferences = WI.settings.resourceContentViewIdentifierForMIMEType.value;
486+
return preferences[mimeType] || null;
487+
}
488+
489+
_setPreferredContentViewIdentifier(identifier)
490+
{
491+
let mimeType = this._getMIMETypeForPreferences();
492+
if (!mimeType)
493+
return;
494+
495+
let preferences = WI.settings.resourceContentViewIdentifierForMIMEType.value;
496+
497+
if (identifier === ResourceClusterContentView.Identifier.Response)
498+
delete preferences[mimeType];
499+
else
500+
preferences[mimeType] = identifier;
501+
502+
// `preferences` is a referenced object. Changing its contents and simply setting it as a new value won't qualify as value change so it won't be persisted.
503+
WI.settings.resourceContentViewIdentifierForMIMEType.value = structuredClone(preferences);
504+
}
505+
466506
_tryEnableCustomRequestContentViews()
467507
{
468508
let content = this._resource.requestData;
@@ -503,13 +543,13 @@ WI.ResourceClusterContentView = class ResourceClusterContentView extends WI.Clus
503543
_tryEnableCustomResponseContentViews()
504544
{
505545
if (!this._resource.hasResponse())
506-
return;
546+
return Promise.resolve();
507547

508548
// WebSocket resources already use a "custom" response content view.
509549
if (this._resource instanceof WI.WebSocketResource)
510-
return;
550+
return Promise.resolve();
511551

512-
this._resource.requestContent()
552+
return this._resource.requestContent()
513553
.then(({error, content}) => {
514554
if (error || typeof content !== "string")
515555
return;

0 commit comments

Comments
 (0)