Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose a filtered content-encoding value to resource timing #411

Open
wants to merge 4 commits into
base: gh-pages
Choose a base branch
from

Conversation

guohuideng2024
Copy link

@guohuideng2024 guohuideng2024 commented Dec 13, 2024

Related PR to fetch specification: whatwg/fetch#1796
Bug: #381


Preview | Diff

index.html Outdated
@@ -715,6 +716,13 @@ <h3>
<a data-for="PerformanceResourceTiming">resource info</a>'s
[=response body info/content type=].
</p>
<p data-dfn-for="PerformanceResourceTiming">
The <dfn>contentEncoding</dfn> getter steps are to return a filtered
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filtered how? The filtering should be done in fetch

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a more detailed and updated filtering description.

I looked at the "get, decode, and split" example in the fetch spec, it looks to me that the result is either a null or a non-empty list. So I used the phrase "null" instead of "empty list". I hope this is correct.

Also should I add my name to "4.10"?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 16, 2025
This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 16, 2025
This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6098321
Commit-Queue: Guohui Deng <guohuideng@microsoft.com>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407331}
aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 16, 2025
This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6098321
Commit-Queue: Guohui Deng <guohuideng@microsoft.com>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407331}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 16, 2025
This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6098321
Commit-Queue: Guohui Deng <guohuideng@microsoft.com>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407331}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jan 21, 2025
…ourceTiming, a=testonly

Automatic update from web-platform-tests
Expose contentEncoding in PerformanceResourceTiming

This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6098321
Commit-Queue: Guohui Deng <guohuideng@microsoft.com>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407331}

--

wpt-commits: 1df2c3e47bcb6379ecf3a07735bd967101d02a5b
wpt-pr: 50115
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Jan 24, 2025
…ourceTiming, a=testonly

Automatic update from web-platform-tests
Expose contentEncoding in PerformanceResourceTiming

This CL introduce a contentEncoding field to Performance resource timing
object. This field is behind a feature flag.

PR to resource timing specification:
w3c/resource-timing#411
PR to fetch specification:
whatwg/fetch#1796

Bug: 327941462
Change-Id: I70cad190fe658fb3dbf8b401ff8393bc1d0782f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6098321
Commit-Queue: Guohui Deng <guohuideng@microsoft.com>
Reviewed-by: Noam Rosenthal <nrosenthal@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Yoav Weiss (@Shopify) <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407331}

--

wpt-commits: 1df2c3e47bcb6379ecf3a07735bd967101d02a5b
wpt-pr: 50115
Copy link
Contributor

@noamr noamr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! See comments.

@@ -715,6 +716,57 @@ <h3>
<a data-for="PerformanceResourceTiming">resource info</a>'s
[=response body info/content type=].
</p>
<p data-dfn-for="PerformanceResourceTiming">
The <dfn>contentEncoding</dfn> getter steps are to perform the following
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contentEncoding getter steps are the following:

body info/content encoding=].
</p>
<p>
<i>codings</i> is a list of strings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this line

</li>
<li>
<p>
<i>codings</i> must contain exactly one string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's obvious from the previous line, not needed

Comment on lines +750 to +754
Let <i>coding</i> be the string in <i>codings</i>.
</p>
If <i>coding</i> is recognized and supported by browser, return
<i>coding</i>.
</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If codings[0] ... is supported by the user agent, and is listed in (... the IANA link... )

</p>
<p class='note'>
A browser should only recognize the empty string or <i>coding</i> values registered
<a href="https://www.iana.org/assignments/http-parameters/http-parameters.xhtml#content-coding">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this should be in the normative section.
You can say in the informative note that "identity" is never supported as a coding and point to the second link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants