Bug 224900 – Web Inspector: Network: rename "XHR" to "XHR/Fetch"

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

[Maximiliano Firtman]

When you want to filter network requests in the Network panel and see only requests done by the fetch API you have to select the filter "XHR"

While XMLHttpRequest is a big part of front-end history and it's still available, the Fetch API is now replacing it. In my experience as a trainer, I've found lately that many web developers with 2 years of experience in the field don't recognize XHR as something they understand as they've never dealt with XHR requests using that API.

It may be a good time to rename the filter to "Fetch" or at least something like "XHR/Fetch"

[Devin Rousso]

Created attachment 426883 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/77288691>

[Patrick Angle]

Comment on attachment 426883 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:66
> +        function createScopeBarItem(id, label, checker) {

I think `pushScopeBarItem` or `addScopeBarItem` would be a more accurate name for this function. `create...` makes me think it's going to only create the item, not also add it to the scope bar.

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
> +        createScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);

Should `Fetch` really be localized since it's the name of an API?

[Patrick Angle]

Comment on attachment 426883 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
>> +        createScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);
> 
> Should `Fetch` really be localized since it's the name of an API?

I realize my question here is slightly ambiguous... Rather, shouldn't we not localize `fetch` because it is API, similar to `XHR`? It seems odd to localize one and not the other given how much they have in common.

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:221
> +            return WI.UIString("fetch", "fetch @ Network Tab Resource Type Column Value", "Shown in the 'Type' column of the Network Table for resources loaded via the 'fetch' method.");

Ditto :76

[Devin Rousso]

Comment on attachment 426883 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
>>> +        createScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);
>> 
>> Should `Fetch` really be localized since it's the name of an API?
> 
> I realize my question here is slightly ambiguous... Rather, shouldn't we not localize `fetch` because it is API, similar to `XHR`? It seems odd to localize one and not the other given how much they have in common.

Hmm, that's a valid question.  I'd personally rather change that in a separate patch tho, as there's probably similar things like this elsewhere in Web Inspector that we could/should localize.

[Devin Rousso]

Created attachment 427631 [details]
Patch

[Patrick Angle]

Comment on attachment 427631 [details]
Patch

LGTM, provisional r+.

[Timothy Hatcher]

Comment on attachment 427631 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:206
> +            return WI.unlocalizedString("css");

Why lowercase on all these? I wouldn't change the case.

[Devin Rousso]

Comment on attachment 427631 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:206
>> +            return WI.unlocalizedString("css");
> 
> Why lowercase on all these? I wouldn't change the case.

These we're all made lowercase at the callsite (old line 204).  I feel like it's safer to have localizers do a translation of the lowercase value rather than rely on JavaScript to do it for us.

[Timothy Hatcher]

Comment on attachment 427631 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
> +        addScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);

Localization would just prefer this to not be a format string, I think. Why not just make this "XHR/Fetch" in the loc?

[Timothy Hatcher]

Comment on attachment 427631 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:206
>>> +            return WI.unlocalizedString("css");
>> 
>> Why lowercase on all these? I wouldn't change the case.
> 
> These we're all made lowercase at the callsite (old line 204).  I feel like it's safer to have localizers do a translation of the lowercase value rather than rely on JavaScript to do it for us.

Ah, eww.

[Devin Rousso]

Comment on attachment 427631 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
>> +        addScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);
> 
> Localization would just prefer this to not be a format string, I think. Why not just make this "XHR/Fetch" in the loc?

I was trying to match the existing behavior of "XHR" not being localized.  I'm fine to change this, but I'd personally rather have that be a followup as I'm sure there's other places this (or things like it) are used that we may want to localize as well.

[Timothy Hatcher]

Comment on attachment 427631 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
>> +        addScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);
> 
> Localization would just prefer this to not be a format string, I think. Why not just make this "XHR/Fetch" in the loc?

If the reason is to not localize XHR and allow Fetch to be localized, Fetch is a technical term too, and likely would be unlocalized.

[Devin Rousso]

Comment on attachment 427631 [details]
Patch

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

>>>> Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js:76
>>>> +        addScopeBarItem("xhr-fetch", WI.UIString("%s/Fetch", "%s/Fetch @ Network Tab Table Filter", "Scope bar button that filter for dynamic resource loads, like from the 'fetch' method.").format(WI.unlocalizedString("XHR")), (type) => type === WI.Resource.Type.XHR || type === WI.Resource.Type.Fetch);
>>> 
>>> Localization would just prefer this to not be a format string, I think. Why not just make this "XHR/Fetch" in the loc?
>> 
>> I was trying to match the existing behavior of "XHR" not being localized.  I'm fine to change this, but I'd personally rather have that be a followup as I'm sure there's other places this (or things like it) are used that we may want to localize as well.
> 
> If the reason is to not localize XHR and allow Fetch to be localized, Fetch is a technical term too, and likely would be unlocalized.

We localize "Fetch" elsewhere in Web Inspector (e.g. `WI.repeatedUIString.fetch`).  I was trying to preserve that behavior too.  tbh all of the situations like this probably need to be reexamined <https://webkit.org/b/225608>

[EWS]

Committed r277289 (237552@main): <https://commits.webkit.org/237552@main>

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