Skip to content

GitLab

T
tangled
Commit d0578d4a authored by David Grogan's avatar David Grogan Committed by Commit Bot
Browse files
Options

[LayoutNG] LayoutNG table-caption implementation 

New passes fast/writing-mode/table-hit-test.html and
tables/mozilla_expected_failures/marvin* had been failing in LayoutNG
because the captions were not centered above the table, for some reason.

The rebased test fast/table/multiple-captions-display-expected.png
failed due to differences on the edges of text inside the caption.

New failure accessibility/table-caption.html fails because the js
accessibility controller can no longer see the caption as a table child,
for some reason.

Making tables/mozilla_expected_failures/bugs/bug3166-16.html pass will
need some digging. Legacy and NG before this patch centered the caption
above the table. But after this patch, it is moved left. Further,
hovering over the caption triggers a DCHECK in the hit testing stack, so
maybe we need to set an offset after all.

ng_physical_fragment.h(144)] Check failed: is_placed_.
blink::NGPhysicalFragment::Offset()
blink::NGPaintFragment::Offset()
blink::NGBoxFragmentPainter::NodeAtPoint()
blink::NGBlockFlowPainter::NodeAtPoint()
blink::LayoutNGMixin<>::NodeAtPoint()
blink::LayoutTable::NodeAtPoint()

Bug: 635619
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I7fcb33e791a9f0d812674a5f7f63cb5964c02a6e
Reviewed-on: https://chromium-review.googlesource.com/1050414
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557632}
parent 9e191d88
Show whitespace changes
Inline Side-by-side
Showing with 166 additions and 17 deletions
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
View file @ d0578d4a
......@@ -111,6 +111,7 @@ crbug.com/591099 accessibility/role-attribute.html [ Failure ]
crbug.com/591099 accessibility/role-change.html [ Failure ]
crbug.com/714962 accessibility/selection-events.html [ Failure ]
crbug.com/714962 accessibility/set-selection-link.html [ Failure ]
crbug.com/591099 accessibility/table-caption.html [ Failure ]
crbug.com/591099 accessibility/table-header-column-row.html [ Failure ]
crbug.com/714962 accessibility/table-with-empty-thead-causes-crash.html [ Failure ]
crbug.com/591099 accessibility/table-with-presentation-role.html [ Failure ]
......@@ -247,14 +248,12 @@ crbug.com/591099 external/wpt/acid/acid3/numbered-tests.html [ Crash ]
crbug.com/591099 external/wpt/acid/acid3/test.html [ Crash ]
crbug.com/591099 external/wpt/credential-management/federatedcredential-framed-get.sub.https.html [ Pass ]
crbug.com/591099 external/wpt/credential-management/passwordcredential-framed-get.sub.https.html [ Pass ]
crbug.com/635619 external/wpt/css/CSS2/floats/floats-in-table-caption-001.html [ Failure ]
crbug.com/714962 external/wpt/css/CSS2/linebox/vertical-align-baseline-005a.xht [ Failure ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-insert-001e.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-insert-001h.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-insert-002e.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-nested-002.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-remove-006.xht [ Pass ]
crbug.com/635619 external/wpt/css/CSS2/normal-flow/margin-collapsing-in-table-caption-002.html [ Failure ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/root-box-001.xht [ Failure ]
crbug.com/591099 external/wpt/css/CSS2/text/white-space-mixed-003.xht [ Pass ]
crbug.com/714962 external/wpt/css/css-backgrounds/background-attachment-local/attachment-local-clipping-color-5.html [ Failure ]
......@@ -1444,7 +1443,6 @@ crbug.com/591099 fast/writing-mode/logical-height-after-clear.html [ Failure ]
crbug.com/591099 fast/writing-mode/percentage-height-orthogonal-writing-modes.html [ Failure ]
crbug.com/591099 fast/writing-mode/percentage-margins-absolute-replaced.html [ Failure ]
crbug.com/591099 fast/writing-mode/percentage-margins-absolute.html [ Failure ]
crbug.com/591099 fast/writing-mode/table-hit-test.html [ Failure ]
crbug.com/591099 fast/writing-mode/table-percent-width-quirk.html [ Failure ]
crbug.com/591099 fast/writing-mode/table-vertical-child-width.html [ Failure ]
crbug.com/591099 fast/writing-mode/text-combine-compress.html [ Failure ]
......@@ -1906,14 +1904,8 @@ crbug.com/591099 tables/mozilla/bugs/bug88035-2.html [ Failure ]
crbug.com/591099 tables/mozilla/bugs/bug88524.html [ Failure ]
crbug.com/591099 tables/mozilla/bugs/bug98196.html [ Failure ]
crbug.com/591099 tables/mozilla/core/table_heights.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/bugs/bug3166-16.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/bugs/bug7113.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_bottom.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_hidden.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_hidden_table.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_left.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_right.html [ Failure ]
crbug.com/591099 tables/mozilla_expected_failures/marvin/table_overflow_caption_top.html [ Failure ]
crbug.com/591099 transforms/2d/transform-2d.html [ Timeout ]
crbug.com/591099 transforms/3d/general/perspective-non-layer.html [ Failure ]
crbug.com/591099 transforms/3d/hit-testing/backface-hit-test.html [ Failure ]
......
third_party/WebKit/LayoutTests/TestExpectations
View file @ d0578d4a
......@@ -278,7 +278,6 @@ crbug.com/819617 [ Linux ] fast/text/descent-clip-in-scaled-page.html [ Failure
crbug.com/796943 fast/text/international/shape-across-elements-simple.html [ Failure ]
### virtual/layout_ng/external/wpt/css/CSS2/floats
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-in-table-caption-001.html [ Failure ]
crbug.com/711704 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-rule3-outside-left-002.xht [ Failure ]
crbug.com/711704 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-rule3-outside-right-002.xht [ Failure ]
crbug.com/711704 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-rule7-outside-left-001.xht [ Failure ]
......@@ -286,7 +285,6 @@ crbug.com/711704 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-rule7-out
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/floats/floats-wrap-bfc-006.xht [ Failure ]
### virtual/layout_ng/external/wpt/css/CSS2/normal-flow
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/margin-collapsing-in-table-caption-002.html [ Failure ]
crbug.com/704961 [ Mac ] virtual/layout_ng/external/wpt/css/CSS2/normal-flow/width-inherit-001.xht [ Failure ]
# Inline: border in continuations. Fail in Blink.
......
third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/table-caption-is-containing-block-001.html 0 → 100644
View file @ d0578d4a
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="David Grogan" href="dgrogan@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/tables.html#model">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="flags" content="" />
<meta name="assert" content="relpos caption is containing block for abspos child" />
<title>
Captions and abspos descendants
</title>
<style>
caption, div {
height:100px;
width:50px;
background:green;
}
#redSquare {
height: 100px;
width: 100px;
background-color: red;
position: absolute;
z-index: -1;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div id="redSquare"></div>
<table>
<caption style="position:relative">
<div style="position:absolute; left:50px;">
</div>
</caption>
</table>
third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/table-caption-passes-abspos-up-001.html 0 → 100644
View file @ d0578d4a
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="David Grogan" href="dgrogan@chromium.org">
<link rel="help" href="https://www.w3.org/TR/CSS22/tables.html#model">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<meta name="flags" content="" />
<meta name="assert" content="non-containing block caption passes abspos child to its parent" />
<title>
Captions and abspos descendants
</title>
<style>
caption, div {
height:100px;
width:50px;
background:green;
}
#redSquare {
height: 100px;
width: 100px;
background-color: red;
position: absolute;
z-index: -1;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div id="redSquare"></div>
<table style="position:relative">
<caption>
<div style="position:absolute; left:50px;">
</div>
</caption>
</table>
third_party/blink/renderer/core/layout/BUILD.gn
View file @ d0578d4a
......@@ -367,6 +367,8 @@ blink_core_sources("layout") {
"ng/layout_ng_block_flow.h",
"ng/layout_ng_mixin.cc",
"ng/layout_ng_mixin.h",
"ng/layout_ng_table_caption.cc",
"ng/layout_ng_table_caption.h",
"ng/layout_ng_table_cell.cc",
"ng/layout_ng_table_cell.h",
"ng/legacy_layout_tree_walking.cc",
......
third_party/blink/renderer/core/layout/layout_object.cc
View file @ d0578d4a
......@@ -78,6 +78,7 @@
#include "third_party/blink/renderer/core/layout/layout_theme.h"
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_block_flow.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_table_cell.h"
#include "third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h"
#include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
......@@ -266,6 +267,8 @@ LayoutObject* LayoutObject::CreateObject(Element* element,
return new LayoutNGTableCell(element);
return new LayoutTableCell(element);
case EDisplay::kTableCaption:
if (ShouldUseNewLayout(style))
return new LayoutNGTableCaption(element);
return new LayoutTableCaption(element);
case EDisplay::kWebkitBox:
case EDisplay::kWebkitInlineBox:
......
third_party/blink/renderer/core/layout/layout_table_caption.h
View file @ d0578d4a
......@@ -35,24 +35,25 @@ class LayoutTable;
//
// This class is very simple as the logic for handling the caption is done in
// LayoutTable. In particular, the placement and sizing is done in
// LayoutTable::layoutCaption. The function is called at different timing
// LayoutTable::LayoutCaption. The function is called at different timing
// depending on the 'caption-side' property: "top" is laid out before table row
// groups when "bottom" ones are laid out after. This ensures that "top"
// captions are visually before the row groups and "bottom" ones are after.
//
// See http://www.w3.org/TR/CSS21/tables.html#caption-position for the
// positioning.
class LayoutTableCaption final : public LayoutBlockFlow {
class LayoutTableCaption : public LayoutBlockFlow {
public:
explicit LayoutTableCaption(Element*);
~LayoutTableCaption() override;
LayoutUnit ContainingBlockLogicalWidthForContent() const override;
private:
protected:
bool IsOfType(LayoutObjectType type) const override {
return type == kLayoutObjectTableCaption || LayoutBlockFlow::IsOfType(type);
}
private:
void InsertedIntoTree() override;
void WillBeRemovedFromTree() override;
......
third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc
View file @ d0578d4a
......@@ -277,6 +277,7 @@ PositionWithAffinity LayoutNGMixin<Base>::PositionForPoint(
return Base::CreatePositionWithAffinity(0);
}
template class LayoutNGMixin<LayoutTableCaption>;
template class LayoutNGMixin<LayoutTableCell>;
template class LayoutNGMixin<LayoutBlockFlow>;
......
third_party/blink/renderer/core/layout/ng/layout_ng_mixin.h
View file @ d0578d4a
......@@ -7,6 +7,7 @@
#include <type_traits>
#include "third_party/blink/renderer/core/layout/layout_table_caption.h"
#include "third_party/blink/renderer/core/layout/layout_table_cell.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_data.h"
#include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
......@@ -95,6 +96,8 @@ class CORE_TEMPLATE_CLASS_EXPORT LayoutNGMixin : public Base {
friend class NGBaseLayoutAlgorithmTest;
};
extern template class CORE_EXTERN_TEMPLATE_EXPORT
LayoutNGMixin<LayoutTableCaption>;
extern template class CORE_EXTERN_TEMPLATE_EXPORT
LayoutNGMixin<LayoutTableCell>;
extern template class CORE_EXTERN_TEMPLATE_EXPORT
......
third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.cc 0 → 100644
View file @ d0578d4a
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.h"
#include "third_party/blink/renderer/core/layout/layout_analyzer.h"
#include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
namespace blink {
LayoutNGTableCaption::LayoutNGTableCaption(Element* element)
: LayoutNGMixin<LayoutTableCaption>(element) {}
void LayoutNGTableCaption::UpdateBlockLayout(bool relayout_children) {
LayoutAnalyzer::BlockScope analyzer(*this);
DCHECK(!IsOutOfFlowPositioned()) << "Out of flow captions are blockified.";
scoped_refptr<NGConstraintSpace> constraint_space =
NGConstraintSpace::CreateFromLayoutObject(*this);
scoped_refptr<NGLayoutResult> result =
NGBlockNode(this).Layout(*constraint_space);
// Tell legacy layout there were abspos descendents we couldn't place. We know
// we have to pass up to legacy here because this method is legacy's entry
// point to LayoutNG. If our parent were LayoutNG, it wouldn't have called
// UpdateBlockLayout, it would have packaged this LayoutObject into
// NGBlockNode and called Layout on that.
for (NGOutOfFlowPositionedDescendant descendant :
result->OutOfFlowPositionedDescendants())
descendant.node.UseOldOutOfFlowPositioning();
// The parent table sometimes changes the caption's position after laying it
// out. So there's no point in setting the fragment's offset here;
// NGBoxFragmentPainter::Paint will have to handle it until table layout is
// implemented in NG, in which case that algorithm will set each child's
// offsets. See https://crbug.com/788590 for more info.
DCHECK(!result->PhysicalFragment()->IsPlacedByLayoutNG())
<< "Only a table should be placing table caption fragments and the ng "
"table algorithm doesn't exist yet!";
}
} // namespace blink
third_party/blink/renderer/core/layout/ng/layout_ng_table_caption.h 0 → 100644
View file @ d0578d4a
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_TABLE_CAPTION_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_TABLE_CAPTION_H_
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/layout/layout_table_caption.h"
#include "third_party/blink/renderer/core/layout/ng/layout_ng_mixin.h"
namespace blink {
class CORE_EXPORT LayoutNGTableCaption final
: public LayoutNGMixin<LayoutTableCaption> {
public:
explicit LayoutNGTableCaption(Element*);
void UpdateBlockLayout(bool relayout_children) override;
const char* GetName() const override { return "LayoutNGTableCaption"; }
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_LAYOUT_NG_TABLE_CAPTION_H_
third_party/blink/tools/blinkpy/web_tests/controllers/single_test_runner.py
View file @ d0578d4a
......@@ -357,13 +357,21 @@ class SingleTestRunner(object):
text = text.replace(char, '')
return text
def remove_ng_text(results):
processed = re.sub(r'LayoutNG(BlockFlow|ListItem|TableCell)', r'Layout\1', results)
# LayoutTableCaption doesn't override LayoutBlockFlow::GetName, so
# render tree dumps have "LayoutBlockFlow" for captions.
processed = re.sub('LayoutNGTableCaption', 'LayoutBlockFlow', processed)
return processed
def is_ng_name_mismatch(expected, actual):
if 'LayoutNGBlockFlow' not in actual:
return False
if not self._is_render_tree(actual) and not self._is_layer_tree(actual):
return False
processed = re.sub(r'LayoutNG(BlockFlow|ListItem|TableCell)', r'Layout\1', actual)
return not self._port.do_text_results_differ(expected, processed)
# There's a mix of NG and legacy names in both expected and actual,
# so just remove NG from both.
return not self._port.do_text_results_differ(remove_ng_text(expected), remove_ng_text(actual))
# LayoutNG name mismatch (e.g., LayoutBlockFlow vs. LayoutNGBlockFlow)
# is treated as pass
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment