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

Update explainer to include units for requestedSize and renderSize code examples. #1145

Merged

Conversation

xiaochen-z
Copy link
Contributor

@xiaochen-z xiaochen-z commented Apr 19, 2024

The requestedSize is a dictionary member of the browserSignals argument to the generateBid function. The implementation of requestedSize appends the size units when constructing it in browserSignals. However, the current explainer does not include units in the code example.

Similarly for renderSize of the browserSignals argument to the scoreAd function, it has not been implemented yet but we plan to append the unit as well. This is currently in progress in this CL.

Also the "height" and "width" in these examples should be strings. The missing quotation marks are added. See spec for requestedSize. Its type is record<DOMString, DOMString>.

Related spec PR #1141 has been approved.

@qingxinwu
Copy link
Collaborator

qingxinwu commented Apr 19, 2024

(just some context for reviewer): See #1141 (comment) for more details.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 19, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 22, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
@xiaochen-z xiaochen-z changed the title Update explainer to include units for requested size and render size Update explainer to include units for requestedSize and renderSize code examples. Apr 22, 2024
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 22, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 22, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 29, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
@xiaochen-z
Copy link
Contributor Author

@JensenPaul This PR is ready for review. Thanks.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 1, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 1, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447954
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294870}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 1, 2024
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447954
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294870}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 6, 2024
…ze field to browserSignals of scoreAd., a=testonly

Automatic update from web-platform-tests
Protected Audience: Add missing renderSize field to browserSignals of
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447954
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294870}

--

wpt-commits: 3c8f88bb2b952e67e70a7a896ef6b0cf259603f0
wpt-pr: 45802
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this pull request May 10, 2024
…ze field to browserSignals of scoreAd., a=testonly

Automatic update from web-platform-tests
Protected Audience: Add missing renderSize field to browserSignals of
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447954
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294870}

--

wpt-commits: 3c8f88bb2b952e67e70a7a896ef6b0cf259603f0
wpt-pr: 45802
@JensenPaul JensenPaul merged commit 36bf1d0 into WICG:main Jun 3, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Jun 3, 2024
…de examples. (#1145)

SHA: 36bf1d0
Reason: push, by JensenPaul

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Jun 6, 2024
…ze field to browserSignals of scoreAd., a=testonly

Automatic update from web-platform-tests
Protected Audience: Add missing renderSize field to browserSignals of
scoreAd.

The Protected Audience explainer claims the browserSignals argument of
scoreAd function should contain an optional field of renderSize.
However, it is found that this has not been implemented. See:
https://github.com/WICG/turtledove/blob/main/FLEDGE.md#:~:text=%27renderSize%27%3A%20%7Bwidth%3A%20100%2C%20height%3A%20200%7D%2C%20/*%20if%20specified%20in%20the%20bid%20*/

This CL implements this optional field to match what the explainer
claims.

See: WICG/turtledove#1088

Spec PR: WICG/turtledove#1141
Explainer PR: WICG/turtledove#1145

Bug: 333628467
Change-Id: I95d5ebb7ddbbeb50d0ac3618e22ed1e228ec32a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5447954
Reviewed-by: Caleb Raitto <caraitto@chromium.org>
Commit-Queue: Xiaochen Zhou <xiaochenzh@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1294870}

--

wpt-commits: 3c8f88bb2b952e67e70a7a896ef6b0cf259603f0
wpt-pr: 45802
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.

None yet

3 participants