Merge pull request #448 from jpzwarte/fix/435-shift-inline-styles

fix: inline styles were not being shifted to custom properties

Author: jgerigmeyer

position-area.html

@@ -183,6 +183,29 @@ <h2>
       </div>
     </section>
 
+    <section class="position-area-demo-item" id="inline-shifted">
+      <h2>
+        <a href="#inline-shifted" aria-hidden="true">🔗</a>
+        <code>span-left top, padding set inline ✅</code>
+      </h2>
+      <div style="position: relative" class="demo-elements">
+        <div class="anchor">Anchor</div>
+        <div class="target inline-shifted" style="padding-right: 50%">
+          Target with longer content
+        </div>
+      </div>
+      <p>
+        The same as the demo above, except that
+        <code>padding-right: 50%</code> is an inline style rather than a
+        stylesheet rule. Inline styles are shifted into custom properties like
+        the rest of the CSS, so <a href="?auto"><code>auto</code> mode</a> can
+        still see the percentage padding and wraps the target. Without that
+        shift the padding reads back as empty, the target is positioned
+        directly, and the padding resolves against the original containing block
+        instead of the <code>position-area</code> cell.
+      </p>
+    </section>
+
     <section class="position-area-demo-item" id="nested-alignment">
       <h2>
         <a href="#nested-alignment" aria-hidden="true">🔗</a>

public/position-area-page.css

@@ -67,6 +67,12 @@
   position-area: span-left top;
 }
 
+/* Same as `.spanleft-top`, but the containing-block-dependent padding is set
+ * as an inline style on the target instead. */
+.target.inline-shifted {
+  position-area: span-left top;
+}
+
 .target.spanall-left {
   position-area: span-all left;
 }

src/fetch.ts

@@ -1,6 +1,6 @@
 import { nanoid } from 'nanoid/non-secure';
 
-import { POLYFILLED_STYLE_ATTRIBUTE } from './cascade.js';
+import { POLYFILLED_STYLE_ATTRIBUTE, SHIFTED_PROPERTIES } from './cascade.js';
 import { querySelectorAllRoots } from './dom.js';
 import {
   type AnchorPositioningRoot,
@@ -63,44 +63,71 @@ async function fetchLinkedStylesheets(
   return results.filter((loaded) => loaded !== null);
 }
 
-const ELEMENTS_WITH_INLINE_ANCHOR_STYLES_QUERY = '[style*="anchor"]';
-const ELEMENTS_WITH_INLINE_POSITION_AREA = '[style*="position-area"]';
-// Searches for all elements with inline style attributes that include `anchor`.
-// For each element found, adds a new 'data-has-inline-styles' attribute with a
-// random UUID value, and then formats the styles in the same manner as CSS from
-// style tags.
+// Inline styles are collected so that `cascadeCSS` can shift their declarations
+// into custom properties, like it does for the rest of the CSS. That has to
+// cover every property the polyfill later reads back through
+// `getCSSPropertyValue` — insets, margins, sizing, padding, self-alignment,
+// `position-area` — and not just the anchor-specific ones: a target can take
+// its `position-area` from a stylesheet while setting its margin inline.
+// `anchor` is matched on its own as well, for `anchor()`/`anchor-size()` values.
+//
+// Matching tests the `style` attribute against a single regex rather than
+// handing `querySelectorAll` one `[style*="..."]` clause per property. Engines
+// do not bucket attribute-substring selectors by attribute presence, so a
+// ~50-clause query runs every substring test against every element in the
+// document; querying `[style]` and filtering here is an order of magnitude
+// faster, and scales with the number of styled elements rather than with the
+// size of the document.
+//
+// Built on first use rather than at module evaluation: `cascade.js` and this
+// module are part of an import cycle, so `SHIFTED_PROPERTIES` is not
+// necessarily initialized yet when this module is evaluated.
+let inlineAnchorStylesRegex: RegExp | undefined;
+/**
+ * Checks if the given element has inline styles used by the polyfill, including
+ * margin, inset, sizing, padding, self-alignment, `position-area`, and anchor
+ * properties.
+ *
+ * @param el The element to check.
+ * @returns True if the element has inline styles used by the polyfill.
+ */
+export function hasInlineAnchorStyles(el: HTMLElement) {
+  if (!inlineAnchorStylesRegex) {
+    // While there is overlap in the terms (`margin` and `margin-block-start`),
+    // reducing the list to only the shortest distinct terms doesn't
+    // significantly improve performance.
+    const terms = ['anchor', ...Object.keys(SHIFTED_PROPERTIES)];
+    // Match at a declaration boundary, so a term appearing in a *value* does
+    // not count: `float: left` and `line-height: 1.5` are not styles we read.
+    inlineAnchorStylesRegex = new RegExp(
+      `(?:^|;)\\s*(?:${terms.join('|')})`,
+      'i',
+    );
+  }
+  return inlineAnchorStylesRegex.test(el.getAttribute('style') ?? '');
+}
+// Searches for all elements with inline style attributes that contain
+// declarations used by the polyfill. For each element found, adds a new
+// 'data-has-inline-styles' attribute with a random UUID value, and then formats
+// the styles in the same manner as CSS from style tags.
 function fetchInlineStyles(elements?: HTMLElement[]) {
-  const elementsWithInlineAnchorStyles: HTMLElement[] = elements
-    ? elements.filter(
-        (el) =>
-          el instanceof HTMLElement &&
-          (el.matches(ELEMENTS_WITH_INLINE_ANCHOR_STYLES_QUERY) ||
-            el.matches(ELEMENTS_WITH_INLINE_POSITION_AREA)),
-      )
-    : Array.from(
-        document.querySelectorAll(
-          [
-            ELEMENTS_WITH_INLINE_ANCHOR_STYLES_QUERY,
-            ELEMENTS_WITH_INLINE_POSITION_AREA,
-          ].join(','),
-        ),
-      );
+  const elementsWithInlineAnchorStyles: HTMLElement[] = (
+    elements ?? Array.from(document.querySelectorAll<HTMLElement>('[style]'))
+  ).filter((el) => el instanceof HTMLElement && hasInlineAnchorStyles(el));
   const inlineStyles: Partial<StyleData>[] = [];
 
-  elementsWithInlineAnchorStyles
-    .filter((el) => el instanceof HTMLElement)
-    .forEach((el) => {
-      const dataAttribute = 'data-has-inline-styles';
-      // Reuse an existing id rather than minting a new one each run: a
-      // concurrent run (e.g. another shadow root being polyfilled) may already
-      // be relying on this element's id in an anchor selector, and re-stamping
-      // it would invalidate that selector.
-      const selector = el.getAttribute(dataAttribute) ?? nanoid(12);
-      el.setAttribute(dataAttribute, selector);
-      const styles = el.getAttribute('style');
-      const css = `[${dataAttribute}="${selector}"] { ${styles} }`;
-      inlineStyles.push({ el, css });
-    });
+  elementsWithInlineAnchorStyles.forEach((el) => {
+    const dataAttribute = 'data-has-inline-styles';
+    // Reuse an existing id rather than minting a new one each run: a
+    // concurrent run (e.g. another shadow root being polyfilled) may already
+    // be relying on this element's id in an anchor selector, and re-stamping
+    // it would invalidate that selector.
+    const selector = el.getAttribute(dataAttribute) ?? nanoid(12);
+    el.setAttribute(dataAttribute, selector);
+    const styles = el.getAttribute('style');
+    const css = `[${dataAttribute}="${selector}"] { ${styles} }`;
+    inlineStyles.push({ el, css });
+  });
 
   return inlineStyles;
 }

tests/e2e/position-area.test.ts

@@ -393,6 +393,31 @@ test.describe('with `positionAreaContainingBlock: auto`', () => {
     ).toHaveCount(1);
   });
 
+  test('wraps a target whose containing-block-dependent style is inline', async ({
+    page,
+  }) => {
+    // `#inline-shifted .target` takes its `position-area` from a stylesheet and
+    // sets `padding-right: 50%` inline. Inline styles are shifted into custom
+    // properties like the rest of the CSS, so the percentage padding is still
+    // seen here and the target is wrapped. Without the shift it reads back as
+    // empty and the target is positioned directly.
+    await applyPolyfill(page);
+
+    const section = page.locator('#inline-shifted');
+    const targetWrapper = section.locator('polyfill-position-area');
+    await expect(targetWrapper).toHaveCount(1);
+
+    // The reason it needs the wrapper: the padding has to resolve against the
+    // position-area cell, not the original parent.
+    const wrapperContentWidth = await targetWrapper.evaluate(
+      (el) => el.clientWidth,
+    );
+    const paddingRight = await section
+      .locator('.target')
+      .evaluate((el) => parseFloat(getComputedStyle(el).paddingRight));
+    expect(paddingRight).toBeCloseTo(wrapperContentWidth / 2, 0);
+  });
+
   test('positions a wrapped target correctly', async ({ page }) => {
     await applyPolyfill(page);
     const section = page.locator('#spanleft-top');

tests/unit/fetch.test.ts

@@ -1,6 +1,6 @@
 import fetchMock from 'fetch-mock';
 
-import { fetchCSS } from '../../src/fetch.js';
+import { fetchCSS, hasInlineAnchorStyles } from '../../src/fetch.js';
 import { getSampleCSS, requestWithCSSType } from '../helpers.js';
 
 describe('fetch stylesheet', () => {
@@ -246,3 +246,80 @@ describe('fetch styles manually', () => {
     expect(styleData[3].css).toContain('top: anchor(--anchor bottom);');
   });
 });
+
+describe('hasInlineAnchorStyles', () => {
+  function elWithStyle(style: string) {
+    const el = document.createElement('div');
+    el.setAttribute('style', style);
+    return el;
+  }
+
+  it('returns false when the element has no style attribute', () => {
+    const el = document.createElement('div');
+    expect(hasInlineAnchorStyles(el)).toBe(false);
+  });
+
+  it('returns false for an empty style attribute', () => {
+    expect(hasInlineAnchorStyles(elWithStyle(''))).toBe(false);
+  });
+
+  it.each([
+    ['color', 'color: red;'],
+    ['background', 'background: blue;'],
+    ['font-weight', 'font-weight: bold;'],
+    ['display', 'display: flex;'],
+    ['z-index', 'z-index: 1;'],
+    ['clear', 'clear: both;'],
+    ['vertical-align', 'vertical-align: middle;'],
+    ['letter-spacing', 'letter-spacing: 1px;'],
+    ['box-sizing', 'box-sizing: border-box;'],
+  ])(
+    'returns false for %s, which is unrelated to the polyfill',
+    (_name, style) => {
+      expect(hasInlineAnchorStyles(elWithStyle(style))).toBe(false);
+    },
+  );
+
+  // Don't match terms that appear in other property names or values.
+  it.each([
+    ['border-top (contains "top")', 'border-top: 1px solid red;'],
+    ['border-left-width (contains "left")', 'border-left-width: 2px;'],
+    ['line-height (contains "height")', 'line-height: 1.5;'],
+    ['float: left (contains "left")', 'float: left;'],
+    ['text-align: right (contains "right")', 'text-align: right;'],
+    ['outline-width (contains "width")', 'outline-width: 1px;'],
+    [
+      'background-position: top (contains "top")',
+      'background-position: top right;',
+    ],
+    ['column-width (contains "width")', 'column-width: 100px;'],
+    ['transform-origin: top left', 'transform-origin: top left;'],
+    ['term as custom property', '--anchor: anchor(--my-anchor);'],
+  ])('returns false for %s', (_name, style) => {
+    expect(hasInlineAnchorStyles(elWithStyle(style))).toBe(false);
+  });
+
+  it.each([
+    ['anchor()', 'top: anchor(--my-anchor end);'],
+    ['anchor-name', 'anchor-name: --my-anchor;'],
+    ['anchor-scope', 'anchor-scope: --my-anchor;'],
+    ['position-anchor', 'position-anchor: --my-anchor;'],
+    ['position-area', 'position-area: top;'],
+    ['an inset longhand', 'inset-block-start: 1px;'],
+    ['a plain inset property', 'top: 1px;'],
+    ['a margin longhand', 'margin-inline-start: 1px;'],
+    ['a plain margin property', 'margin-left: 1px;'],
+    ['a sizing property', 'width: 100px;'],
+    ['a min-sizing longhand', 'min-inline-size: 100px;'],
+    ['a padding longhand', 'padding-inline-start: 1px;'],
+    ['a plain padding property', 'padding: 1px;'],
+    ['a self-alignment property', 'justify-self: center;'],
+  ])('returns true when the style includes %s', (_name, style) => {
+    expect(hasInlineAnchorStyles(elWithStyle(style))).toBe(true);
+  });
+
+  it('matches regardless of where the relevant declaration falls', () => {
+    const el = elWithStyle('color: red; anchor-name: --my-anchor; z-index: 1;');
+    expect(hasInlineAnchorStyles(el)).toBe(true);
+  });
+});