Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
API changes to make FLEDGE understand ad sizes #417
API changes to make FLEDGE understand ad sizes #417
Changes from 1 commit
1a2368f
ff42d5b
d63d8e1
8851afc
d662fb7
038f2ca
7791d78
397e544
19c08f5
bbc82b5
0251bbc
3ef5be6
fc9653f
01ef829
a0a310d
1ddddee
d642777
f25854c
d119ceb
5380368
d102fbc
47d50e6
568873a
0a3db48
adf4cbc
cb668e8
733a821
3ec71ac
cfbcb70
7c6aa0e
85c2f60
1e07855
c1e00cf
2d44dd9
7625240
9ecc5f6
4efd2e8
064841f
8199508
81a8bf9
7b856f7
0cc20cb
21795b9
ed3b830
cd73790
06ace19
ca80dd8
910a9f2
3170266
b65b04a
4d4364b
6e6ec85
975e965
c3fad7a
cce7780
3e34e0d
a3662d5
d69f233
2d3f228
a57adbe
7148d74
f8cd3f6
50893b4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current Chrome implementation all sizes have to have units ('px', 'sq', or 'sh'). Is it intended to allow unitless numbers as a shortcut for "px"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does our implementation support "sh" as a valid unit? It seems only "sw" and "px" are supported. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brusshamilton It is intended to allow unitless numbers as a shortcut for "px", but currently doesn't.
@qingxinwu Lack of "sh" is also a known issue.
We will fix both of these problems imminently (i.e. for M114), thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly a silly question, but where are
sw
orsh
defined? I don't see them in https://drafts.csswg.org/css-values-4/#viewport-relative-units (are they some magic ads thing)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not silly at all: they aren't CSS units. As far as we can tell (I consulted with some screen/layout experts), there are no existing units relative to the screen dimensions, rather than window etc. dimensions (which don't satisfy the privacy properties we need, namely that they can't be modified by the web platform).
This is nice in that we aren't beholden to some existing behavior specification; we can make the behavior exactly what is necessary for FLEDGE privacy/utility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, interesting - thanks! I assume we'll define those at some later point in the spec.