Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Table with fixed layout behaves like auto layout when its width is se…
…t by JS instead of css

Table with fixed layout behaves like auto layout when its width is set by JS instead of css
https://bugs.webkit.org/show_bug.cgi?id=130239

Reviewed by Alan Baradlay.

This patch is to align WebKit with Blink / Chromium and Gecko / Firefox.

Merge - https://chromium.googlesource.com/chromium/blink/+/17dc4cd863992d6f9932ba40d5cf41a1389dfa97

The new width of table was not being applied when changed dynamically
even though the table-layout is fixed.

On calculating whether we need to set the m_tableLayout as fixed or
auto the change in layout parameters such as width, height etc was
not considered hence the right layout type was not being set. Incase
there are any layout changes we need to see whether m_tableLayout
needs to be changed or not.

* Source/WebCore/rendering/RenderTable.cpp:
(RenderTable::styleDidChange): Update as per commit message
* Source/WebCore/rendering/style/RenderStyle.h: Add bool 'isFixedTableLayout' with return value
* LayoutTests/fast/table/fixed-table-layout-width-change.html: Add Test Case
* LayoutTests/fast/table/fixed-table-layout-width-change-expected.txt: Add Test Case Expectation

Canonical link: https://commits.webkit.org/260143@main
  • Loading branch information
Ahmad-S792 authored and Ahmad Saleem committed Feb 11, 2023
1 parent d43e2a2 commit b6c7186
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
@@ -0,0 +1,2 @@
Tests that the table width having table-layout fixed changes when width is changed dynamically.
PASS
32 changes: 32 additions & 0 deletions LayoutTests/fast/table/fixed-table-layout-width-change.html
@@ -0,0 +1,32 @@
<!DOCTYPE html>
<html>
<head>
<script type="text/javascript">
function runTest() {
var outer = document.getElementById('outer');
outer.style.width = '100px';
window.checkLayout("#outer", document.getElementById("test-output"));
};
</script>
<script src="../../resources/check-layout.js"></script>
<style>
#outer {
display: table;
table-layout: fixed;
}
#inner {
display: table-cell;
height: 50px;
background-color: green;
min-width: 200px;
}
</style>
</head>
<body onload="runTest()">
Tests that the table width having table-layout fixed changes when width is changed dynamically.
<div id="outer" data-expected-width="100">
<div id="inner"></div>
</div>
<div id="test-output"></div>
</body>
</html>
10 changes: 5 additions & 5 deletions Source/WebCore/rendering/RenderTable.cpp
Expand Up @@ -4,8 +4,8 @@
* (C) 1998 Waldo Bastian (bastian@kde.org)
* (C) 1999 Lars Knoll (knoll@kde.org)
* (C) 1999 Antti Koivisto (koivisto@kde.org)
* Copyright (C) 2003-2022 Apple Inc. All rights reserved.
* Copyright (C) 2015 Google Inc. All rights reserved.
* Copyright (C) 2003-2023 Apple Inc. All rights reserved.
* Copyright (C) 2014-2015 Google Inc. All rights reserved.
* Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
*
* This library is free software; you can redistribute it and/or
Expand Down Expand Up @@ -130,17 +130,17 @@ void RenderTable::styleDidChange(StyleDifference diff, const RenderStyle* oldSty
RenderBlock::styleDidChange(diff, oldStyle);
propagateStyleToAnonymousChildren(PropagateToAllChildren);

auto oldTableLayout = oldStyle ? oldStyle->tableLayout() : TableLayoutType::Auto;
bool oldFixedTableLayout = oldStyle ? oldStyle->isFixedTableLayout() : false;

// In the collapsed border model, there is no cell spacing.
m_hSpacing = collapseBorders() ? 0 : style().horizontalBorderSpacing();
m_vSpacing = collapseBorders() ? 0 : style().verticalBorderSpacing();
m_columnPos[0] = m_hSpacing;

if (!m_tableLayout || style().tableLayout() != oldTableLayout) {
if (!m_tableLayout || style().isFixedTableLayout() != oldFixedTableLayout) {
// According to the CSS2 spec, you only use fixed table layout if an
// explicit width is specified on the table. Auto width implies auto table layout.
if (style().tableLayout() == TableLayoutType::Fixed && !style().logicalWidth().isAuto())
if (style().isFixedTableLayout())
m_tableLayout = makeUnique<FixedTableLayout>(this);
else
m_tableLayout = makeUnique<AutoTableLayout>(this);
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/rendering/style/RenderStyle.h
Expand Up @@ -2,7 +2,8 @@
* Copyright (C) 2000 Lars Knoll (knoll@kde.org)
* (C) 2000 Antti Koivisto (koivisto@kde.org)
* (C) 2000 Dirk Mueller (mueller@kde.org)
* Copyright (C) 2003-2017 Apple Inc. All rights reserved.
* Copyright (C) 2003-2023 Apple Inc. All rights reserved.
* Copyright (C) 2014 Google Inc. All rights reserved.
* Copyright (C) 2006 Graham Dennis (graham.dennis@gmail.com)
*
* This library is free software; you can redistribute it and/or
Expand Down Expand Up @@ -483,6 +484,7 @@ class RenderStyle {
ListStyleType listStyleType() const { return static_cast<ListStyleType>(m_inheritedFlags.listStyleType); }
StyleImage* listStyleImage() const;
ListStylePosition listStylePosition() const { return static_cast<ListStylePosition>(m_inheritedFlags.listStylePosition); }
bool isFixedTableLayout() const { return tableLayout() == TableLayoutType::Fixed && !logicalWidth().isAuto(); }

const Length& marginTop() const { return m_surroundData->margin.top(); }
const Length& marginBottom() const { return m_surroundData->margin.bottom(); }
Expand Down

0 comments on commit b6c7186

Please sign in to comment.