New issue
Advanced search Search tips

Issue 801040 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

[LayoutNG] NGOffsetMappingBuilder supports generated characters in the middle of a node

Project Member Reported by kojii@chromium.org, Jan 11 2018

Issue description

Discussion from https://chromium-review.googlesource.com/c/chromium/src/+/860962

NGOffsetMappingBuilder assumes no generated character in the middle of a text node.

Now this patch gives a counter example: we may have BiDi control chars around a '\n' in a white-space: pre-line text node.

Need a way to get around this...
 
Cc: -xiaoche...@chromium.org
Owner: xiaoche...@chromium.org
Status: Assigned (was: Available)
I'm working on this.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 12 2018

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

commit 490029f265606592c20c68a4e8d0db7551ec3b9e
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Fri Jan 12 04:24:32 2018

[LayoutNG] Allow inconsecutive appending to NGOffsetMappingBuilder

NGOffsetMappingBuilder used to assume that characters of a text node must
be appended consecutively. However, a previous patch (crrev.com/c/860962)
introduced a pattern where generated BiDi control characters can be added
in the middle of a text node. To make NGOffsetMappingBuilder work again:

1. NGOffsetMappingBuilder::Build() is modified to handle the case where
   characters of the same source node are appended inconsecutively, by
   remembering the last seen non-null source node and the number of chars
   in it that have been processed

2. To make the source node marking easier, NGOffsetMapping is changed to
   use a scope-like object SourceNodeScope, so that characters appended
   in a scope are automatically marked with a sourced node given by the
   scope.

Bug:  801040 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Id95b579fa4b84d7347dbc4e580ad6f5665b7e1ae
Reviewed-on: https://chromium-review.googlesource.com/862524
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528877}
[modify] https://crrev.com/490029f265606592c20c68a4e8d0db7551ec3b9e/third_party/WebKit/Source/core/layout/ng/inline/empty_offset_mapping_builder.h
[modify] https://crrev.com/490029f265606592c20c68a4e8d0db7551ec3b9e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_items_builder.cc
[modify] https://crrev.com/490029f265606592c20c68a4e8d0db7551ec3b9e/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_builder.cc
[modify] https://crrev.com/490029f265606592c20c68a4e8d0db7551ec3b9e/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_builder.h
[modify] https://crrev.com/490029f265606592c20c68a4e8d0db7551ec3b9e/third_party/WebKit/Source/core/layout/ng/inline/ng_offset_mapping_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment