Reland "[anchor-position] Support margin properties in position fallb…

…ack" This reverts commit 426a7d8727ff4a2767fdd3cb22b5f091888aa39c. Reason for reland: ensured roundtrip for simple values - Reverted behavior (PS#1): check layout for all boxes - Old behavior (base): check layout for boxes with non-fixed style values - New behavior: check layout for boxes with either non-fixed style values or using position fallback style Original change's description: > Revert "[anchor-position] Support margin properties in position fallback" > > This reverts commit 502912cd04e83f9d1ce1d8df50fc9778b57207e0. > > Reason for revert: causing https://crbug.com/1485377 > > Original change's description: > > [anchor-position] Support margin properties in position fallback > > > > This patch makes margin properties (except margin-trim) allowed in a > > @position-fallback rule, so that different fallback positions can use > > different margins. > > > > There's also some changes to `getComputedStyle()` implementation to > > make it return the used margin values from fallback styles. To do so, > > we unconditionally obtain the result from a `LayoutBox` if the current > > object is a box. Since the result is available only after layout, this > > patch also changes all margin properties on boxes to be layout > > dependent, regardless of the computed margin values. > > > > Note: This causes some rounding differences, as the result from > > `LayoutBox`, which is the actual layout precision, has a fixed-point > > precision of 1/64 px. Therefore, some tests are rebaselined. > > > > Bug: 1475317 > > Change-Id: I4a3e242ceb04413db5348770b5cb666a54a1c9e0 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4819910 > > Reviewed-by: Ian Kilpatrick <[email protected]> > > Reviewed-by: Sami Kyöstilä <[email protected]> > > Commit-Queue: Xiaocheng Hu <[email protected]> > > Cr-Commit-Position: refs/heads/main@{#1190240} > > Bug: 1475317, 1485377 > Change-Id: Ieb200c75919ad7ad67745519791fcb9473c24e9b > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4881060 > Reviewed-by: Sami Kyöstilä <[email protected]> > Commit-Queue: Xiaocheng Hu <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1199388} Bug: 1475317, 1485377 Change-Id: I95789a405a2e1bc71ec3b3819a8bb6c4ddd77ad1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4885150 Commit-Queue: Xiaocheng Hu <[email protected]> Auto-Submit: Xiaocheng Hu <[email protected]> Reviewed-by: Ian Kilpatrick <[email protected]> Cr-Commit-Position: refs/heads/main@{#1203515}
web-platform-tests · Sep 29, 2023 · b695a02 · b695a02
1 parent a1b7cb7
commit b695a02
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 7 deletions.
diff --git a/css/css-anchor-position/at-fallback-position-allowed-declarations.html b/css/css-anchor-position/at-fallback-position-allowed-declarations.html
@@ -45,6 +45,19 @@
 test_allowed_declaration('inset-inline');
 test_allowed_declaration('inset');
 
+// Margin properties are allowed
+test_allowed_declaration('margin-top');
+test_allowed_declaration('margin-bottom');
+test_allowed_declaration('margin-left');
+test_allowed_declaration('margin-right');
+test_allowed_declaration('margin-block-start');
+test_allowed_declaration('margin-block-end');
+test_allowed_declaration('margin-inline-start');
+test_allowed_declaration('margin-inline-end');
+test_allowed_declaration('margin-block');
+test_allowed_declaration('margin-inline');
+test_allowed_declaration('margin');
+
 // Sizing properties are allowed
 test_allowed_declaration('width');
 test_allowed_declaration('height');
@@ -70,13 +83,6 @@
 // Custom properties are disallowed
 test_disallowed_declaration('--custom');
 
-// Margin properties are disallowed
-test_disallowed_declaration('margin-left');
-test_disallowed_declaration('margin-right');
-test_disallowed_declaration('margin-top');
-test_disallowed_declaration('margin-bottom');
-test_disallowed_declaration('margin');
-
 // Test some other disallowed properties
 test_disallowed_declaration('font-size');
 test_disallowed_declaration('border-width');

diff --git a/css/css-anchor-position/position-fallback-004.html b/css/css-anchor-position/position-fallback-004.html
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+<title>Tests margin properties in position fallback</title>
+<link rel="help" href="https://drafts.csswg.org/css-anchor-position-1/#accepted-try-properties">
+<link rel="author" href="mailto:[email protected]">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/check-layout-th.js"></script>
+<script src="support/test-common.js"></script>
+
+<style>
+body {
+  margin: 0;
+}
+
+.cb {
+  width: 300px;
+  height: 150px;
+  position: relative;
+  background: lightgray;
+}
+
+.anchor {
+  position: absolute;
+  width: 100px;
+  height: 100px;
+  top: 25px;
+  background: orange;
+  anchor-name: --a;
+}
+
+.target {
+  position: absolute;
+  width: 100px;
+  height: 100px;
+  background: lime;
+  position-fallback: --fallbacks;
+}
+
+@position-fallback --fallbacks {
+  @try {
+    top: anchor(--a top);
+    right: anchor(--a left);
+    margin-top: 10px;
+    margin-right: 10px;
+  }
+
+  @try {
+    bottom: anchor(--a bottom);
+    left: anchor(--a right);
+    margin-bottom: 10px;
+    margin-left: 10px;
+  }
+}
+</style>
+
+<body onload="checkLayoutForAnchorPos('.target')">
+
+<div class=cb>
+  <div class=anchor style="left: 110px"></div>
+  <!-- Chooses 1st @try block. -->
+  <div class=target data-offset-x=0
+       data-expected-margin-left=0 data-expected-margin-right=10
+       data-expected-margin-top=10 data-expected-margin-bottom=0></div>
+</div>
+
+<div class=cb>
+  <div class=anchor style="right: 110px"></div>
+  <!-- Chooses 2nd @try block. -->
+  <div class=target data-offset-x=200
+       data-expected-margin-left=10 data-expected-margin-right=0
+       data-expected-margin-top=0 data-expected-margin-bottom=10></div>
+</div>
+
+</body>
diff --git a/css/cssom/getComputedStyle-margins-roundtrip.html b/css/cssom/getComputedStyle-margins-roundtrip.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<title>Chromium bug: getComputedStyle() fixed-length inset values don't roundtrip</title>
+<link rel="help" href="https://drafts.csswg.org/cssom/#resolved-value">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1482703">
+<link rel="help" href="mailto:[email protected]">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div id="target1" style="margin-left: 20.7px"></div>
+<script>
+test(() => {
+  assert_equals(getComputedStyle(target1).marginLeft, '20.7px');
+}, 'Fixed-length margin-left property value should roundtrip');
+</script>
+
+<div id="target2" style="margin-right: 20.7px"></div>
+<script>
+test(() => {
+  assert_equals(getComputedStyle(target2).marginRight, '20.7px');
+}, 'Fixed-length margin-right property value should roundtrip');
+</script>
+
+<div id="target3" style="margin-top: 20.7px"></div>
+<script>
+test(() => {
+  assert_equals(getComputedStyle(target3).marginTop, '20.7px');
+}, 'Fixed-length margin-top property value should roundtrip');
+</script>
+
+<div id="target4" style="margin-bottom: 20.7px"></div>
+<script>
+test(() => {
+  assert_equals(getComputedStyle(target4).marginBottom, '20.7px');
+}, 'Fixed-length margin-bottom property value should roundtrip');
+</script>