New issue
Advanced search Search tips

Issue 911220 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 8
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

[LayoutNG] DCHECK in NGInlineNode::ComputeOffsetMapping when inspecting font-variant-06.xht

Project Member Reported by bunge...@chromium.org, Dec 3

Issue description

Chromium 73.0.3630.0 (Developer Build) (64-bit)
Revision 9887251b9a16ab4651666766fb5717fc9ba8497e-refs/heads/master@{#613120}
OS Linux

What steps will reproduce the problem?
(1) With debug LayoutNG build, open third_party/blink/web_tests/external/wpt/css/css-fonts/font-variant-06.xht
(2) Open the inspector
(3) Expand 'head' and then 'style'

What is the expected result?
No crash.

What happens instead?
[1:1:1203/141136.345812:FATAL:ng_inline_node.cc(320)] Check failed: data->text_content == builder.ToString() (

"\uFFFC\uFFFC\u2066  @font-face {\n    font-family: fwf;\n    src: url(support/fonts/FontWithFancyFeatures.otf);\n    font-variant: oldstyle-nums;\n  }\n  .test, .ref {\n    font-family: fwf;\n    font-size: 2.4em;\n    position: absolute;\n  }\n  .test {\n      color: green;\n  }\n  .ref {\n      color: red;\n  }\n  .t1 { }\n  .t2 { font-variant-numeric: oldstyle-nums; }\n  .t3 { font-variant-numeric: normal; }\n  .t4 { font-variant-numeric: slashed-zero; }\n  .t5 { font-variant-numeric: invalid-value; }\n  .t6 { font-variant: none; }\n  .t7 { font-variant: normal; }\n  .t8 { font-variant: initial; }\n  .t9 { font-variant: invalid-value; }\n  /**\n   * Some notes on this test.\n   *\n   * t1\n   *   the font-variant property from @font-face is applied.\n   * t2\n   *   simply restating the value set in the @font-face rule\n   * t3\n   *   \"font-variant-numeric: normal\" does not turn off the \"on\" value\n   *   for oldstyle-nums, which remains on as set in @font-face\n   * t4\n   *   \"font-variant-numeric: slashed-zero\" is valid, but does not\n   *   change the \"on\" value for oldstyle-nums as set in @font-face\n   * t6, t7, t8\n       these font-variant values all set font-variant-numeric to normal\n   * t5, r9\n   *   an invalid value means the rule is invalid, and should be ignored.\n   */\n\u2069"
vs.
"\uFFFC\uFFFC\u2066  @font-face {\u2069\n\u2066    font-family: fwf;\u2069\n\u2066    src: url(support/fonts/FontWithFancyFeatures.otf);\u2069\n\u2066    font-variant: oldstyle-nums;\u2069\n\u2066  }\u2069\n\u2066  .test, .ref {\u2069\n\u2066    font-family: fwf;\u2069\n\u2066    font-size: 2.4em;\u2069\n\u2066    position: absolute;\u2069\n\u2066  }\u2069\n\u2066  .test {\u2069\n\u2066      color: green;\u2069\n\u2066  }\u2069\n\u2066  .ref {\u2069\n\u2066      color: red;\u2069\n\u2066  }\u2069\n\u2066  .t1 { }\u2069\n\u2066  .t2 { font-variant-numeric: oldstyle-nums; }\u2069\n\u2066  .t3 { font-variant-numeric: normal; }\u2069\n\u2066  .t4 { font-variant-numeric: slashed-zero; }\u2069\n\u2066  .t5 { font-variant-numeric: invalid-value; }\u2069\n\u2066  .t6 { font-variant: none; }\u2069\n\u2066  .t7 { font-variant: normal; }\u2069\n\u2066  .t8 { font-variant: initial; }\u2069\n\u2066  .t9 { font-variant: invalid-value; }\u2069\n\u2066  /**\u2069\n\u2066   * Some notes on this test.\u2069\n\u2066   *\u2069\n\u2066   * t1\u2069\n\u2066   *   the font-variant property from @font-face is applied.\u2069\n\u2066   * t2\u2069\n\u2066   *   simply restating the value set in the @font-face rule\u2069\n\u2066   * t3\u2069\n\u2066   *   \"font-variant-numeric: normal\" does not turn off the \"on\" value\u2069\n\u2066   *   for oldstyle-nums, which remains on as set in @font-face\u2069\n\u2066   * t4\u2069\n\u2066   *   \"font-variant-numeric: slashed-zero\" is valid, but does not\u2069\n\u2066   *   change the \"on\" value for oldstyle-nums as set in @font-face\u2069\n\u2066   * t6, t7, t8\u2069\n\u2066       these font-variant values all set font-variant-numeric to normal\u2069\n\u2066   * t5, r9\u2069\n\u2066   *   an invalid value means the rule is invalid, and should be ignored.\u2069\n\u2066   */\u2069\n\u2066\u2069"
)


Please use labels and text to provide additional information.

The difference appears to be that builder.ToString() has returned a string with a number of unicode directional override codepoints (particularly \u2069 and \u2066) which aren't in the text_content (and aren't in the file).
 
Note that this seems somewhat like  issue 879088 , but the only similarity seems to be that the same dcheck caught the issue. In this case the issue is directional overrides showing up.
Owner: xiaoche...@chromium.org
Status: Assigned (was: Untriaged)
I'll take a look for NGOffsetMapping.
It's due to a preserved '\n' in bidi context handled incorrectly in re-layout.

In case of such '\n', we can't reuse inline items in re-layout, because we need to exit and enter all bidi contexts around the '\n'.

Failing to do that causes text mismatch in re-layout and offset mapping computation.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 8

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

commit 66f151dcc4ae6276efc3c48812069671907fefba
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Sat Dec 08 03:34:03 2018

Don't reuse inline items if there is preserved newline in bidi context

For preserved newlines in bidi context, we need to pop all contexts
before it and re-enter after it. To ensure to same popping and re-
entering behavior, we can't reuse the inline items in re-layout.

Bug:  911220 
Change-Id: I5d687e89e289283843f6f9ed32d028df50fc1a1a
Reviewed-on: https://chromium-review.googlesource.com/c/1368577
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614937}
[modify] https://crrev.com/66f151dcc4ae6276efc3c48812069671907fefba/third_party/blink/renderer/core/layout/ng/inline/ng_inline_items_builder.cc
[modify] https://crrev.com/66f151dcc4ae6276efc3c48812069671907fefba/third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment