Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/45739.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Hello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Hello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
You're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
You're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
David BaronYou're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
I can't really recommend implementing your own interpretation of "padding", it must surely cause some inconsistencies with other parts of CSS.
I assume we can't just impose some limit on the padding (in `em`) in this case?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
David BaronYou're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
Anders Hartvoll Ruud(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
I can't really recommend implementing your own interpretation of "padding", it must surely cause some inconsistencies with other parts of CSS.
I assume we can't just impose some limit on the padding (in `em`) in this case?
Vertical padding could be limited by `em` but horizontal padding max size depends on the content-size, which is why we implemented `min|max-width` with `calc-size`.
So ignoring height for a second, since that can be bound by `1em`:
The ultimate goal is to ensure that whatever padding is added to the element, the total width (border+padding+content) doesn't run over the `max-width` (which is 3*`fit-content` as defined in the previous CL). Also as an addition, the `span` element should always be centered in the `permission` element so padding should not be allowed to push the span to the side or out of the bounds of the `permission` element. That is why I basically want to put an upper bound on `padding-left|right` which is equal to the equivalent of k * `fit-content`. (Right now k = 1, but that could change in the future)
I have tried playing around with `box-sizing: border-box` but that doesn't seem to put actual limits on the padding width: if `padding-left` + `padding-right` > `max-width` the element grows over max-width anyways. I've played around both with the `<permission>` element and divs and spans, but like I said I couldn't get it to work, if you have a working example I would be more than happy to chase that approach further. (I've also found various conflicting information on the web on whether `box-sizing: border-box` interacts correctly with `max-width` so I'm not exactly sure if it works reliably).
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
David BaronYou're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
Anders Hartvoll Ruud(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
Andy PaicuI can't really recommend implementing your own interpretation of "padding", it must surely cause some inconsistencies with other parts of CSS.
I assume we can't just impose some limit on the padding (in `em`) in this case?
Vertical padding could be limited by `em` but horizontal padding max size depends on the content-size, which is why we implemented `min|max-width` with `calc-size`.
So ignoring height for a second, since that can be bound by `1em`:
The ultimate goal is to ensure that whatever padding is added to the element, the total width (border+padding+content) doesn't run over the `max-width` (which is 3*`fit-content` as defined in the previous CL). Also as an addition, the `span` element should always be centered in the `permission` element so padding should not be allowed to push the span to the side or out of the bounds of the `permission` element. That is why I basically want to put an upper bound on `padding-left|right` which is equal to the equivalent of k * `fit-content`. (Right now k = 1, but that could change in the future)
I have tried playing around with `box-sizing: border-box` but that doesn't seem to put actual limits on the padding width: if `padding-left` + `padding-right` > `max-width` the element grows over max-width anyways. I've played around both with the `<permission>` element and divs and spans, but like I said I couldn't get it to work, if you have a working example I would be more than happy to chase that approach further. (I've also found various conflicting information on the web on whether `box-sizing: border-box` interacts correctly with `max-width` so I'm not exactly sure if it works reliably).
What are the underlying goals of the minimum and maximum sizes here? Is the purpose of the minimum size (for both width and height) is to ensure that the text inside the element is visible? What's the purpose of the maximum size?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
David BaronYou're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
Anders Hartvoll Ruud(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
Andy PaicuI can't really recommend implementing your own interpretation of "padding", it must surely cause some inconsistencies with other parts of CSS.
I assume we can't just impose some limit on the padding (in `em`) in this case?
David BaronVertical padding could be limited by `em` but horizontal padding max size depends on the content-size, which is why we implemented `min|max-width` with `calc-size`.
So ignoring height for a second, since that can be bound by `1em`:
The ultimate goal is to ensure that whatever padding is added to the element, the total width (border+padding+content) doesn't run over the `max-width` (which is 3*`fit-content` as defined in the previous CL). Also as an addition, the `span` element should always be centered in the `permission` element so padding should not be allowed to push the span to the side or out of the bounds of the `permission` element. That is why I basically want to put an upper bound on `padding-left|right` which is equal to the equivalent of k * `fit-content`. (Right now k = 1, but that could change in the future)
I have tried playing around with `box-sizing: border-box` but that doesn't seem to put actual limits on the padding width: if `padding-left` + `padding-right` > `max-width` the element grows over max-width anyways. I've played around both with the `<permission>` element and divs and spans, but like I said I couldn't get it to work, if you have a working example I would be more than happy to chase that approach further. (I've also found various conflicting information on the web on whether `box-sizing: border-box` interacts correctly with `max-width` so I'm not exactly sure if it works reliably).
What are the underlying goals of the minimum and maximum sizes here? Is the purpose of the minimum size (for both width and height) is to ensure that the text inside the element is visible? What's the purpose of the maximum size?
Minimum is to ensure the text is inside and is visible indeed.
Maximum is to ensure that the text is clearly labeling the button, otherwise you can imagine a huge button stretched over the entire view-port and the button text being just a relatively small surface area inside that button. It would be possible for users to not perceive it as the label to a button.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
David BaronHello Anders and David, PTAL.
David, am I right about the fact that `calc-size` can't be used in padding?
Anders, PTAL at the other CSS stuff.
David BaronYou're correct that `calc-size()` can't be used in `padding`. But I'm not entirely sure what it is that you wanted to do with it. `calc()` can be used in `padding`. Would that have worked?
Or, taking an entirely different approach, would `box-sizing: border-box` have worked? (That does expose a slightly different developer-facing API, but that seems like it might be justified in this case if it helps the restrictions here make sense.)
Anders Hartvoll Ruud(That said, I think it would help me understand the general approach here if I could see a statement of what you're intending to allow vs. forbid with this change.)
Andy PaicuI can't really recommend implementing your own interpretation of "padding", it must surely cause some inconsistencies with other parts of CSS.
I assume we can't just impose some limit on the padding (in `em`) in this case?
David BaronVertical padding could be limited by `em` but horizontal padding max size depends on the content-size, which is why we implemented `min|max-width` with `calc-size`.
So ignoring height for a second, since that can be bound by `1em`:
The ultimate goal is to ensure that whatever padding is added to the element, the total width (border+padding+content) doesn't run over the `max-width` (which is 3*`fit-content` as defined in the previous CL). Also as an addition, the `span` element should always be centered in the `permission` element so padding should not be allowed to push the span to the side or out of the bounds of the `permission` element. That is why I basically want to put an upper bound on `padding-left|right` which is equal to the equivalent of k * `fit-content`. (Right now k = 1, but that could change in the future)
I have tried playing around with `box-sizing: border-box` but that doesn't seem to put actual limits on the padding width: if `padding-left` + `padding-right` > `max-width` the element grows over max-width anyways. I've played around both with the `<permission>` element and divs and spans, but like I said I couldn't get it to work, if you have a working example I would be more than happy to chase that approach further. (I've also found various conflicting information on the web on whether `box-sizing: border-box` interacts correctly with `max-width` so I'm not exactly sure if it works reliably).
Andy PaicuWhat are the underlying goals of the minimum and maximum sizes here? Is the purpose of the minimum size (for both width and height) is to ensure that the text inside the element is visible? What's the purpose of the maximum size?
Minimum is to ensure the text is inside and is visible indeed.
Maximum is to ensure that the text is clearly labeling the button, otherwise you can imagine a huge button stretched over the entire view-port and the button text being just a relatively small surface area inside that button. It would be possible for users to not perceive it as the label to a button.
Would it be sufficient to:
(That is, would that be enough to avoid having to manipulate the `width` constraints in order to support `padding`?)
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
I think we could do something like that where we guestimate how much padding is appropriate at most (`2em` is too little probably more like `5em`), but I would rather be able to set a limit that is based on content length since that can vary widely based on i18n and permission status.
However, if there is no relatively straightforward solution (e.g. to support `calc-size` in `padding`, which likely has implications I don't understand), then putting a static limit on it seems like the next best thing.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
I don't think supporting `calc-size()` in `padding` is plausible, since what you're asking for there is not `fit-content` for `padding` but `fit-content` for an entirely different property (`width`), which would be a pretty major architectural change (having `calc()` or `calc-size()` expressions accept values that aren't valid for the property).
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Acknowledged, I have switched to fixed sizes for the max bounds. Thank you for confirming.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
👍
the control of the site author), this CL provides a mechanism that
sets the width/height based on padding.
Not accurate anymore?
box-sizing: border-box !important;
This is intentional, right? (It's not mentioned in the commit message).
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Is this still relevant?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
the control of the site author), this CL provides a mechanism that
sets the width/height based on padding.
Andy PaicuNot accurate anymore?
Done
box-sizing: border-box !important;
This is intentional, right? (It's not mentioned in the commit message).
Done, augmented commit message.
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Is this still relevant?
Unfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Andy PaicuIs this still relevant?
Unfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Can you not just using `padding` in the ref as well?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Andy PaicuIs this still relevant?
Anders Hartvoll RuudUnfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Can you not just using `padding` in the ref as well?
If we did that we would no longer be testing anything at all. The 2 elements would be identical style-wise so they would of-course render identically.
I'm trying to ensure that padding actually takes effect so, for example: if padding-left is 10px (and width is auto) then the apparent visual width of the button should be text + 10px on each side. The only way I could think of doing this is to calculate the overall necessary width (fit-content + 2*padding) and set that on the reference element. Then the one with padding should look identical to the one that has the size set fit-content + 2*padding.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Andy PaicuIs this still relevant?
Anders Hartvoll RuudUnfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Andy PaicuCan you not just using `padding` in the ref as well?
If we did that we would no longer be testing anything at all. The 2 elements would be identical style-wise so they would of-course render identically.
I'm trying to ensure that padding actually takes effect so, for example: if padding-left is 10px (and width is auto) then the apparent visual width of the button should be text + 10px on each side. The only way I could think of doing this is to calculate the overall necessary width (fit-content + 2*padding) and set that on the reference element. Then the one with padding should look identical to the one that has the size set fit-content + 2*padding.
Oh yeah, I forgot that it's a `<permission>` element on both sides, i.e. both sides are subjected to the same adjustments. :-)
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Code-Review | +1 |
LGTM as well, although I think it's worth noting that if the end goal is to get this feature shipped in multiple engines, it would be good to get feedback on this sort of approach from the others sooner rather than later, since it's possible that this sort of adjustment mechanism (which is unusual and doesn't match normal UA style adjustments) might not be feasible in other engines (particularly Gecko, since there isn't a common origin for the codebase).
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Andy PaicuIs this still relevant?
Anders Hartvoll RuudUnfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Andy PaicuCan you not just using `padding` in the ref as well?
Anders Hartvoll RuudIf we did that we would no longer be testing anything at all. The 2 elements would be identical style-wise so they would of-course render identically.
I'm trying to ensure that padding actually takes effect so, for example: if padding-left is 10px (and width is auto) then the apparent visual width of the button should be text + 10px on each side. The only way I could think of doing this is to calculate the overall necessary width (fit-content + 2*padding) and set that on the reference element. Then the one with padding should look identical to the one that has the size set fit-content + 2*padding.
Oh yeah, I forgot that it's a `<permission>` element on both sides, i.e. both sides are subjected to the same adjustments. :-)
I'm curious if `width: max-content` or `width: min-content` might have produced better results than `width: fit-content`... but it probably doesn't make a difference.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Commit-Queue | +2 |
LGTM as well, although I think it's worth noting that if the end goal is to get this feature shipped in multiple engines, it would be good to get feedback on this sort of approach from the others sooner rather than later, since it's possible that this sort of adjustment mechanism (which is unusual and doesn't match normal UA style adjustments) might not be feasible in other engines (particularly Gecko, since there isn't a common origin for the codebase).
We are in constant discussion with other vendors, including about the styling restrictions. I can't say the feedback is exactly what I was hoping for, but communication is happening.
Thank you both.
/* There is a slight misalignment of the text (by 1px) when using
padding vs using width/height. Since this test's purpose is to
check that the bounds are identical, the color and background-color
are set to the same value to cover the slight text misalignment */
color:black;
background-color: black;
Andy PaicuIs this still relevant?
Anders Hartvoll RuudUnfortunately, yes.
The "actual" element (in this file) uses padding and the "expected" element (in the reference file) uses fixed width/height to achieve the same overall result. However there is a slight misalignment of the text (1px) between the 2 methods, it seems the padding method results in the text being 1px to the left vs fixed width reference (the overall bounds e.g. border are still identical). This seemed largely superfluous so I went with this approach to make them visually identical (after various unsuccessful attempts to fix it via CSS).
Andy PaicuCan you not just using `padding` in the ref as well?
Anders Hartvoll RuudIf we did that we would no longer be testing anything at all. The 2 elements would be identical style-wise so they would of-course render identically.
I'm trying to ensure that padding actually takes effect so, for example: if padding-left is 10px (and width is auto) then the apparent visual width of the button should be text + 10px on each side. The only way I could think of doing this is to calculate the overall necessary width (fit-content + 2*padding) and set that on the reference element. Then the one with padding should look identical to the one that has the size set fit-content + 2*padding.
David BaronOh yeah, I forgot that it's a `<permission>` element on both sides, i.e. both sides are subjected to the same adjustments. :-)
I'm curious if `width: max-content` or `width: min-content` might have produced better results than `width: fit-content`... but it probably doesn't make a difference.
I have tried `min-content`, but not `max-content` but I doubt that would work since making the width bigger would automatically cause pixel differences.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
[PEPC] Provide partial padding support to allow for common use case
Removal of 'padding' support from the permission element has removed
support for a common use case that used padding in conjunction with
'width: auto'. Since it is very difficult to provide a fixed width
for the permission element (as the text can change and is not under
the control of the site author), this CL provides a mechanism that
sets the width/height based on padding.
We don't want to simply support padding unchecked since it can be
used to push the text outside of the permission element area, and
creating a bounded expression is very difficult because 'calc-size'
is not supported for padding lengths.
Also do a quick cleanup of vertical-align.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/45739
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |