New issue
Advanced search Search tips

Issue 872207 link

Starred by 1 user
Status: Fixed
Owner:
andruud@chromium.org
Closed: Aug 9
Cc:
futhark@chromium.org
style-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[LayoutNG] ::first-letter with legacy fallback should not crash

Project Member Reported by yosin@chromium.org, Aug 8 
Following layout tests are crashed with:
  layout_tree_builder.h(66)] Check failed: node.NeedsAttach()
for first-letter pseudo element.

editing/inserting/insert-character-in-first-letter-crash.html
editing/text-iterator/first-letter-word-boundary.html
editing/text-iterator/read-past-cloned-first-letter.html
fast/css/first-letter-block-form-controls-crash.html
fast/css/first-letter-capitalized-edit-select-crash.html
fast/css/first-letter-crash-document-disposal.html
fast/css/first-letter-text-fragment-crash.html
fast/ruby/ruby-first-letter.html

Comment 1 by yosin@chromium.org, Aug 8 

For  fast/css/first-letter-text-fragment-crash.html, 
document.designMode = 'on' makes <html> element's set ComputedStyle::ForceLegacyLayout() then
ComputedStyle::StylePropagationDiff() returns |kReatach| for <html>

Then RecalcStyle() is called with |kReatch| for <div> with ::first-letter

In legacy, RecalcStyle() for <div> with ::first-letter is called with |kForce| as specified at Document::UpdateStyle().

Comment 2 by andruud@chromium.org, Aug 8

Components: -Blink>Layout Blink>CSS
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 9 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18b2fbc6797c30140e1d7e9988459a426ec4f31e

commit 18b2fbc6797c30140e1d7e9988459a426ec4f31e
Author: Anders Hartvoll Ruud <andruud@chromium.org>
Date: Thu Aug 09 07:15:10 2018

Detach ::first-letter in Element::DetachLayoutTree.

The code for first-letter was recently reworked, but this was overlooked.

Fixes DCHECK failures for LayoutNG.

R=futhark@chromium.org, yosin@chromium.org

Bug:  872207 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I03e7aa3216c3e6c9e8576ff2d76cdb20730a9aae
Reviewed-on: https://chromium-review.googlesource.com/1166921
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581813}
[modify] https://crrev.com/18b2fbc6797c30140e1d7e9988459a426ec4f31e/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/18b2fbc6797c30140e1d7e9988459a426ec4f31e/third_party/blink/renderer/core/dom/element.cc

Comment 4 by andruud@chromium.org, Aug 9

Status: Fixed (was: Started)
►

Sign in to add a comment