Skip to content

Fix position: absolute to reference nearest positioned ancestor#276

Merged
yorkie merged 8 commits intomainfrom
copilot/fix-275
Sep 4, 2025
Merged

Fix position: absolute to reference nearest positioned ancestor#276
yorkie merged 8 commits intomainfrom
copilot/fix-275

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 1, 2025

This PR fixes a critical CSS positioning bug where position: absolute elements were incorrectly positioned relative to the entire page instead of their nearest positioned ancestor, breaking standard web layout behavior.

Problem

Previously, the layout engine treated all absolutely positioned elements as if they were positioned relative to the viewport/page, ignoring intermediate positioned ancestors. This broke expected layout for:

  • Nested components with relative/absolute positioning
  • Modal dialogs and overlays
  • Dropdown menus and tooltips
  • Any UI components using CSS stacking contexts

Solution

Implemented the missing positioning logic in the layout engine according to W3C CSS specifications:

// Before: All TODO methods returned null/false
bool LayoutObject::computeIsAbsoluteContainer(const ComputedStyle &style) const {
  // TODO: implement this method.
  return false;
}

// After: Correctly identifies positioning contexts
bool LayoutObject::computeIsAbsoluteContainer(const ComputedStyle &style) const {
  if (style.hasProperty("position")) {
    auto position = style.getPropertyValue("position");
    return position == "relative" || position == "absolute" || 
           position == "fixed" || position == "sticky";
  }
  return false;
}

The implementation now correctly:

  1. Identifies positioning contexts - Elements with position: relative, absolute, fixed, or sticky establish containing blocks for absolutely positioned descendants
  2. Traverses the DOM hierarchy - Walks up the ancestor chain to find the nearest positioned ancestor
  3. Falls back gracefully - Returns to viewport positioning when no positioned ancestor exists

Example

Consider this HTML structure:

<div style="position: relative">     <!-- Positioned ancestor -->
  <div>                             <!-- Static element -->
    <div style="position: absolute; top: 10px; right: 10px;">
      <!-- This should position relative to the outer div, not the page -->
    </div>
  </div>
</div>

Before this fix: The absolutely positioned element would position relative to the entire page.

After this fix: The absolutely positioned element correctly positions relative to the nearest position: relative ancestor.

Testing

  • Added comprehensive unit tests in layout_position_tests.cpp
  • Created visual test fixture position-absolute-test.html demonstrating correct behavior
  • All existing tests pass with no regressions
  • Code follows project formatting standards (clang-format + ESLint)

This change brings JSAR Runtime's CSS positioning behavior in line with standard web browsers and W3C specifications.

Fixes #275.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • registry.npmmirror.com
    • Triggering command: npm ci (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 4 commits September 1, 2025 13:42
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
…vior

Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix position: absolute to reference nearest positioned ancestor, not entire page Fix position: absolute to reference nearest positioned ancestor Sep 1, 2025
Copilot AI requested a review from yorkie September 1, 2025 13:50
@yorkie yorkie marked this pull request as ready for review September 4, 2025 07:39
@yorkie yorkie merged commit 397aac7 into main Sep 4, 2025
2 checks passed
@yorkie yorkie deleted the copilot/fix-275 branch September 4, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix position: absolute to reference nearest positioned ancestor, not entire page

2 participants