[PEPC] Add tests that check container CSS affects validation [chromium/src : main]
Andy Paicu (Gerrit)
Andy Paicu added 1 comment![]( "Open in Gerrit")
Related details
- Thomas Nguyen
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen added 1 comment![]( "Open in Gerrit")
"rotate(0.1turn)");I need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
Related details
- Andy Paicu
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Paicu (Gerrit)
Andy Paicu added 1 comment![]( "Open in Gerrit")
"rotate(0.1turn)");I need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
The IntersectionObserver only allows non-distorting visual effects such as translations and up-scaling [1]. Rotations are not allowed, probably due to how complicated it would be to compute this. I don't think this should be a problem for us.
Related details
- Thomas Nguyen
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thomas Nguyen
| Code-Review | +1 |
1 comment
"rotate(0.1turn)");Andy PaicuI need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
The IntersectionObserver only allows non-distorting visual effects such as translations and up-scaling [1]. Rotations are not allowed, probably due to how complicated it would be to compute this. I don't think this should be a problem for us.
It seems to be disabled permanently. In this case, users and devs will have no idea why it fails and not be directed to the root cause and how to fix it.
I am not opposing, but a task for us is show the useful/correct information in this case (eg: a log like " the element is not visible" is not relevant)
Related details
- Andy Paicu
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Paicu (Gerrit)
Andy Paicu added 1 comment![]( "Open in Gerrit")
"rotate(0.1turn)");Andy PaicuI need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
Thomas NguyenThe IntersectionObserver only allows non-distorting visual effects such as translations and up-scaling [1]. Rotations are not allowed, probably due to how complicated it would be to compute this. I don't think this should be a problem for us.
It seems to be disabled permanently. In this case, users and devs will have no idea why it fails and not be directed to the root cause and how to fix it.
I am not opposing, but a task for us is show the useful/correct information in this case (eg: a log like " the element is not visible" is not relevant)
Should we expand the console message then to include also transforms?
Related details
- Thomas Nguyen
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thomas Nguyen
| Code-Review | +1 |
1 comment
"rotate(0.1turn)");Andy PaicuI need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
Thomas NguyenThe IntersectionObserver only allows non-distorting visual effects such as translations and up-scaling [1]. Rotations are not allowed, probably due to how complicated it would be to compute this. I don't think this should be a problem for us.
Andy PaicuIt seems to be disabled permanently. In this case, users and devs will have no idea why it fails and not be directed to the root cause and how to fix it.
I am not opposing, but a task for us is show the useful/correct information in this case (eg: a log like " the element is not visible" is not relevant)
Should we expand the console message then to include also transforms?
Yes we should, the idea is, we should show a log when the PEPC is disabled *permanently*. We only had logs on several static checks like security context: permission policy and fenced frame, currently the visibility check is from IntersectionObserver -> we are missing it (or might be misaligned)
Related details
- Andy Paicu
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Paicu (Gerrit)
Andy Paicu added 2 comments![]( "Open in Gerrit")
"rotate(0.1turn)");Andy PaicuI need to understand why this transform will disable the PEPC. This will rotate the container (+ PEPC inside). Following the visibility logic, the PEPC is still visible and should be clickable.
Thomas NguyenThe IntersectionObserver only allows non-distorting visual effects such as translations and up-scaling [1]. Rotations are not allowed, probably due to how complicated it would be to compute this. I don't think this should be a problem for us.
Andy PaicuIt seems to be disabled permanently. In this case, users and devs will have no idea why it fails and not be directed to the root cause and how to fix it.
I am not opposing, but a task for us is show the useful/correct information in this case (eg: a log like " the element is not visible" is not relevant)
Thomas NguyenShould we expand the console message then to include also transforms?
Yes we should, the idea is, we should show a log when the PEPC is disabled *permanently*. We only had logs on several static checks like security context: permission policy and fenced frame, currently the visibility check is from IntersectionObserver -> we are missing it (or might be misaligned)
Since we have a task to add logs for developers that we might have missed. Let's leave it as part of that.
Related details
- Mason Freed
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted Code-Review+1![]( "Open in Gerrit")
- Andy Paicu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andy Paicu (Gerrit)
Andy Paicu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andy Paicu
| Commit-Queue | +2 |
1 comment
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
[PEPC] Add tests that check container CSS affects validation
There are various properties that can be applied to parent elements
that would affect the validity of the PEPC element. These are covered
by the IntersectionObserver visibility logic. This CL only adds tests
to verify the behavior.
Also a small bug fix to the "disable" logic where an indefinite disable
would not override a temporary disable. The scenario is verified in
*.IntersectionChangedDisableEnableDisable
- M third_party/blink/renderer/core/html/html_permission_element.cc
- M third_party/blink/renderer/core/html/html_permission_element_test.cc
Code-Review: +1 by Thomas Nguyen, +1 by Mason Freed
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |