New issue
Advanced search Search tips

Issue 829234 link

Starred by 1 user
Status: Fixed
Owner:
yosin@chromium.org
Closed: May 2018
Cc:
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 832055
issue 832350
issue 832497



Sign in to add a comment

TextOffsetMapping::ComputeContainigBlock() should return block box containing inline content

Project Member Reported by yosin@chromium.org, Apr 5 2018 
Following functions in TextOffsetMapping should return LayoutBlock containing
inline content:
- ComputeContainigBlock()
- NextBlockFor()
- PreviousBlockFor()

We should change:
 - IsBlockForTextOffsetMapping()
 - Change ComputeContaingBlock() to traverse both ancestors then descendants


Example:
ComputeContaingBlock(BODY@0) should return LayoutBlockFlow(P) contains "foo"

<div><p>foo</p><p>bar</p></div>

Layout tree:
  LayoutBlockFlow {HTML} at (0,0) size 800x600
    LayoutBlockFlow {BODY} at (8,8) size 784x576
      LayoutBlockFlow {DIV} at (0,0) size 784x56
        LayoutBlockFlow {P} at (0,0) size 784x20
          LayoutText {#text} at (0,0) size 21x19
            text run at (0,0) width 21: "foo"
        LayoutBlockFlow {P} at (0,36) size 784x20
          LayoutText {#text} at (0,0) size 20x19
            text run at (0,0) width 20: "bar"

Comment 1 by yosin@chromium.org, Apr 5 2018 

IsBlockForTextOffsetMapping() should return true for
 - LayoutObject::ChildrenInline()
 - empty LayoutBlock

See LayoutBlock::AddChild(), accepts only block content
LayoutBlockFlow::AddChild(), accepts both block content and inline content

Comment 2 by xiaoche...@chromium.org, Apr 5 2018 

Is it a change in design?

Comment 3 by yosin@chromium.org, Apr 6 2018 

>#c2 Is it a change in design?
No. TextOffsetMapping should work only for block box contains inline content.

Comment 4 by xiaoche...@chromium.org, Apr 17 2018 

If we want to enforce inline children for TextOffsetMapping's containing block, we should use LayoutBlockFlow instead of LayoutBlock as the containing block's type.

Comment 5 by yosin@chromium.org, Apr 18 2018

Owner: yosin@chromium.org
Status: Started (was: Available)
Project Member

Comment 6 by bugdroid1@chromium.org, May 10 2018 

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

commit 4c414662683ec6083af7de599683c7fc177dc5d6
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Thu May 10 01:29:33 2018

Introduce TextMappingOffset::InlineContents class

This patch introduces |TextMappingOffset::InlineContents| class to represent
a run of inline layout objects with iterator as preparation of computing
inline layout object run from |LayoutBlockFlow|.

Bug:  829234 
Change-Id: If2fc23545b3983c7798553460d1cf06d08dd337c
Reviewed-on: https://chromium-review.googlesource.com/1018923
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557409}
[modify] https://crrev.com/4c414662683ec6083af7de599683c7fc177dc5d6/third_party/blink/renderer/core/editing/text_offset_mapping.cc
[modify] https://crrev.com/4c414662683ec6083af7de599683c7fc177dc5d6/third_party/blink/renderer/core/editing/text_offset_mapping.h
[modify] https://crrev.com/4c414662683ec6083af7de599683c7fc177dc5d6/third_party/blink/renderer/core/editing/text_offset_mapping_test.cc
[modify] https://crrev.com/4c414662683ec6083af7de599683c7fc177dc5d6/third_party/blink/renderer/core/editing/visible_units_word.cc

Comment 7 by yosin@chromium.org, May 10 2018

Status: Fixed (was: Started)
http://crrev.com/c/1018923 should fix this issue.

Comment 8 by awhalley@google.com, May 14 2018

Labels: Merge-Request-67 OS-Linux
requesting merge to 67 as this also fixes  issue 840979
Project Member

Comment 9 by sheriffbot@chromium.org, May 14 2018

Labels: -Merge-Request-67 Merge-Reject-67 Hotlist-Merge-Reject
The bug is marked as P3 or Feature. It should not be merged as M67 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
►

Sign in to add a comment