New issue
Advanced search Search tips

Issue 789870 link

Starred by 0 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocked on: View detail
issue 812535
issue 891169

Blocking:
issue 708452



Sign in to add a comment

Get rid of Frameselection::VisibleSelection from render pipeline

Project Member Reported by yoichio@chromium.org, Nov 30 2017

Issue description

We should replace VisibleSelection canonicalization with
another canonicalization w/o InlineBox.
It might be LayoutSelection canonicalization.

For example, VisibleSelection::IsCaret() is used from following files:
editing/EditingStyle.cpp	 (1 occurrence)
editing/EditingStyleUtilities.cpp	 (1 occurrence)
editing/Editor.cpp	 (3 occurrences)
editing/FrameCaret.cpp	 (1 occurrence)
editing/FrameSelection.cpp	 (5 occurrences)
editing/FrameSelectionTest.cpp	 (1 occurrence)
editing/SelectionModifier.cpp	 (2 occurrences)
editing/VisibleSelection.cpp	 (1 occurrence)
editing/VisibleSelectionTest.cpp	 (2 occurrences)
editing/commands/EditorCommand.cpp	 (7 occurrences)
editing/commands/ReplaceSelectionCommand.cpp	 (1 occurrence)
editing/commands/TypingCommand.cpp	 (6 occurrences)
editing/spellcheck/SpellChecker.cpp	 (1 occurrence)
frame/LocalFrameView.cpp	 (1 occurrence)
layout/LayoutTreeAsText.cpp	 (1 occurrence)
page/DragController.cpp	 (1 occurrence)
svg/UnsafeSVGAttributeSanitizationTest.cpp	 (1 occurrence)



 

Comment 1 by yosin@chromium.org, Jan 10 2018

Labels: Type-Task
Project Member

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

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

commit 549773224d585e74a112ee6be35fe575404dd225
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Jan 17 02:59:48 2018

Refactor RenderPosition::GetLocalSelectionEndpoints

This patch extracts the boolean reference value assignment out from
the function to improve code health.

Bug: 789870
Change-Id: I3e483c501cb4c54889194b776d46636996b3e603
Reviewed-on: https://chromium-review.googlesource.com/867818
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529560}
[modify] https://crrev.com/549773224d585e74a112ee6be35fe575404dd225/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/549773224d585e74a112ee6be35fe575404dd225/third_party/WebKit/Source/core/editing/RenderedPosition.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 17 2018

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

commit f8f09e481cafffc2a71101e913b445f6907297e1
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Jan 17 04:05:00 2018

Refactor RenderPosition::LocalToInvalidationBackingPoint

This patch extracts GraphicsLayer pointer assignment to local function
GetGraphicsLayerBacking to improve code health.

Bug: 789870
Change-Id: I0e491b4b2b7c4fe0dc263f4cedd029476bc8c607
Reviewed-on: https://chromium-review.googlesource.com/867820
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529592}
[modify] https://crrev.com/f8f09e481cafffc2a71101e913b445f6907297e1/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/f8f09e481cafffc2a71101e913b445f6907297e1/third_party/WebKit/Source/core/editing/RenderedPosition.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 17 2018

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

commit 8cea09725b14cdb7e0868272aba2b26ae2ac94cc
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Jan 17 07:20:36 2018

Refactor RenderPosition::PositionInGraphicsLayerBacking

This patch changes the function signature from
assigning to reference value to returning the value for code health.

Bug: 789870
Change-Id: I9a3db1f29b3db72037659e4a81def495f10356ea
Reviewed-on: https://chromium-review.googlesource.com/867821
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529664}
[modify] https://crrev.com/8cea09725b14cdb7e0868272aba2b26ae2ac94cc/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/8cea09725b14cdb7e0868272aba2b26ae2ac94cc/third_party/WebKit/Source/core/editing/RenderedPosition.h
[modify] https://crrev.com/8cea09725b14cdb7e0868272aba2b26ae2ac94cc/third_party/WebKit/Source/core/frame/LocalFrameView.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 18 2018

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

commit b4e65b824752271809f0a461dabe35a1505a07b3
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jan 18 02:39:05 2018

Refactor RenderPosition::PositionInGraphicsLayerBacking #2

This patch moves CompositedSelectionBound.hidden assignment from
LocalFrameView to PositionInGraphicsLayerBacking() for code health.

Bug: 789870
Change-Id: I007aee8b5e6acc78c57127feb4191b94d2789852
Reviewed-on: https://chromium-review.googlesource.com/869697
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530025}
[modify] https://crrev.com/b4e65b824752271809f0a461dabe35a1505a07b3/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/b4e65b824752271809f0a461dabe35a1505a07b3/third_party/WebKit/Source/core/editing/RenderedPosition.h
[modify] https://crrev.com/b4e65b824752271809f0a461dabe35a1505a07b3/third_party/WebKit/Source/core/frame/LocalFrameView.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 18 2018

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

commit 16b6bc1af03ebd94910b2fc1e665d6cee10733fe
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jan 18 03:23:01 2018

Refactor RenderPosition::GetLocalSelectionEndpoints

This patch changes the function signature from
assigning to reference values of two LayoutPoints
to returning the values of std::pair for code health.

Bug: 789870
Change-Id: Ic47dc16598456169af3480b0591c56248cc35b30
Reviewed-on: https://chromium-review.googlesource.com/870014
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530040}
[modify] https://crrev.com/16b6bc1af03ebd94910b2fc1e665d6cee10733fe/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/16b6bc1af03ebd94910b2fc1e665d6cee10733fe/third_party/WebKit/Source/core/editing/RenderedPosition.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 18 2018

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

commit 2a123c2bfe3e7f9711efd7b78f7783b01f127d92
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jan 18 09:43:14 2018

Refactor LocalFrameView::ComputeCompositedSelection

This patch move almost part of the function into the
new function RenderedPosition::ComputeCompositedSelection
for improve code modularity.

Bug: 789870
Change-Id: I3915dd8d93638f9949f4f5c000cfeba2bc204214
Reviewed-on: https://chromium-review.googlesource.com/872690
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530100}
[modify] https://crrev.com/2a123c2bfe3e7f9711efd7b78f7783b01f127d92/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/2a123c2bfe3e7f9711efd7b78f7783b01f127d92/third_party/WebKit/Source/core/editing/RenderedPosition.h
[modify] https://crrev.com/2a123c2bfe3e7f9711efd7b78f7783b01f127d92/third_party/WebKit/Source/core/frame/LocalFrameView.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 19 2018

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

commit e291ba527772d8758503bf495124bfa6f6f3dea2
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Jan 19 09:21:43 2018

Refactor LocalFrameView::ComputeCompositedSelection

This patch makes the function local and change the signature
from assigning to reference argument |CompositedSelection&|
to returning the value.
This patch also improve if-else code flow of the caller
LocalFrameView::UpdateCompositedSelectionIfNeeded.

Bug: 789870
Change-Id: I004142521c5848d5f905e777c519ea653b6e934c
Reviewed-on: https://chromium-review.googlesource.com/875515
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530457}
[modify] https://crrev.com/e291ba527772d8758503bf495124bfa6f6f3dea2/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/e291ba527772d8758503bf495124bfa6f6f3dea2/third_party/WebKit/Source/core/frame/LocalFrameView.h

Description: Show this description
Summary: Create visibles election w/o InlieBox (was: Canonicalize w/o VisibleSelection.)
Summary: Create visible selection w/o InlieBox (was: Create visibles election w/o InlieBox)
Summary: Create visible selection w/o InlineBox (was: Create visible selection w/o InlieBox)
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 25 2018

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

commit b8d8c3414fe2de6c52e594c0e955ae07b3d437f0
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jan 25 02:12:43 2018

Refactor Renderedposition::ComputeCompositedSelection using LocalCaretRect

This patch refactors the function replacing
LayoutObject::LocalCaretRect(InlineBox, int offset)
with LocalCaretRectOfPosition(PositionWithAffinity) for NG.

Since RenderedPosition stores a InlineBox and computes LayoutRect
in its instance functions, this patch makes the functions static
and receive LocalCaretRect.
As a result, this patch makes ComputeCompositedSelection independent
from RenderedPosition class.

This patch also remove GetSamplePointForVisibility test
because it is now local function only used by IsVisible
which is test inside ComputeCompositedSelection test;

Note:
CompositedSelectionBoundsTest also tests ComputeCompositedSelection

Bug: 789870
Change-Id: If955d56deac1477e65d71523fb628d75b421b2ab
Reviewed-on: https://chromium-review.googlesource.com/875842
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531788}
[modify] https://crrev.com/b8d8c3414fe2de6c52e594c0e955ae07b3d437f0/third_party/WebKit/Source/core/editing/RenderedPosition.cpp
[modify] https://crrev.com/b8d8c3414fe2de6c52e594c0e955ae07b3d437f0/third_party/WebKit/Source/core/editing/RenderedPosition.h
[modify] https://crrev.com/b8d8c3414fe2de6c52e594c0e955ae07b3d437f0/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 25 2018

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

commit b8d03518573f750870aeda01d79ce4b7ae0ab71a
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Jan 25 06:01:57 2018

[LayoutNG] Move a local function to resolve forward declaration.

This patch moves PositionInGraphicsLayerBacking() after IsVisible().

Bug: 789870
Change-Id: Ia4c7bcd590ea36cfefc97ae3f92eca01142304f8
Reviewed-on: https://chromium-review.googlesource.com/885665
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531824}
[modify] https://crrev.com/b8d03518573f750870aeda01d79ce4b7ae0ab71a/third_party/WebKit/Source/core/editing/RenderedPosition.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 27 2018

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

commit 47aba8bb1ceac8d39b93fb0ed6c53d129afc1b8d
Author: Yoichi Osato <yoichio@chromium.org>
Date: Sat Jan 27 00:19:43 2018

[LayoutNG] Refactor RenderedPosition::IsVisible not to use boolean argument.

Since the argument |selection_start| is used to compute two LayoutPoints,
this patch salvages them from IsVibisile to the caller CompositedSelectionBound
which already has the LayoutPoints.

Bug: 789870
Change-Id: I43e751d35268466e910ac826820010a719a8298e
Reviewed-on: https://chromium-review.googlesource.com/888338
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532098}
[modify] https://crrev.com/47aba8bb1ceac8d39b93fb0ed6c53d129afc1b8d/third_party/WebKit/Source/core/editing/RenderedPosition.cpp

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 29 2018

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

commit ba7f41458c92a3b249157cb94c8db39f68b8465d
Author: Yoichi Osato <yoichio@chromium.org>
Date: Mon Jan 29 09:25:42 2018

[LayoutNG] Refactor RenderedPosition::ComputeCompositedSelection

This patch changes the function to use a SelectionInDOMTree instance
rather than VisibleSelection one.
This patch is a preparation for removing VS canonicalization:
crrev.com/c/872811(WIP)

Bug: 789870
Change-Id: I38388e01b3262bc3a8abf455dfb9f566ef9cf510
Reviewed-on: https://chromium-review.googlesource.com/890574
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532367}
[modify] https://crrev.com/ba7f41458c92a3b249157cb94c8db39f68b8465d/third_party/WebKit/Source/core/editing/RenderedPosition.cpp

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 29 2018

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

commit 437234455aa6dcaac3bc1b3ee7f975322ea1337b
Author: Yoichi Osato <yoichio@chromium.org>
Date: Mon Jan 29 09:36:31 2018

[LayoutNG] Refactor RenderedPosition::GetLocalSelectionEndpoints

GetLocalSelectionEndpoints uses a boolean argument |selection_start| to compute
a selection bound.
This patch refactors the function by removing the argument and splitting
the function into GetLocalSelectionStartpoints and
GetLocalSelectionEndpoints for code health.

Bug: 789870
Change-Id: I7a2f3e7ab9f086c286c0f60bd1e8ce8a1d1c94a8
Reviewed-on: https://chromium-review.googlesource.com/890695
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532370}
[modify] https://crrev.com/437234455aa6dcaac3bc1b3ee7f975322ea1337b/third_party/WebKit/Source/core/editing/RenderedPosition.cpp

Project Member

Comment 18 by bugdroid1@chromium.org, Jan 30 2018

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

commit 267cd8de179e5344aafff5d67fbb8a7430f7688f
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Jan 30 06:00:22 2018

[LayoutNG] Refactor RenderedPosition::PositionInGraphicsLayerBacking

PositionInGraphicsLayerBacking uses a boolean argument |selection_start| to compute
a selection bound.
This patch refactors the function by removing the argument and splitting
the function into StartPositionInGraphicsLayerBacking and
EndPositionInGraphicsLayerBacking for code health.

Bug: 789870
Change-Id: I09b08afaef179cedd00d6d9b6bbd47cb5cd222bb
Reviewed-on: https://chromium-review.googlesource.com/892193
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532786}
[modify] https://crrev.com/267cd8de179e5344aafff5d67fbb8a7430f7688f/third_party/WebKit/Source/core/editing/RenderedPosition.cpp

Blockedon: 812535
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 2

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

commit e91ca1605c75ec3dcc93f5f3b4764897b36db2ff
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Aug 02 06:20:56 2018

[LayoutNG] Remove unused variable in layout_block_flow_line.cc

This is a follow-up from:
https://chromium-review.googlesource.com/c/chromium/src/+/583977
which removed unused InlineBox::HasSelectedChildren().
The patch left an unused variable |root_has_selected_children|
dangling.
This patch removed that.

Bug: 789870
Change-Id: I6f5d4cf4f50630e558a314782212949ccd5ad2bc
Reviewed-on: https://chromium-review.googlesource.com/1159555
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580089}
[modify] https://crrev.com/e91ca1605c75ec3dcc93f5f3b4764897b36db2ff/third_party/blink/renderer/core/layout/layout_block_flow_line.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 6

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

commit 440ed9790705720ebe88bdd76ba6ea1668424379
Author: Yoichi Osato <yoichio@chromium.org>
Date: Mon Aug 06 05:23:00 2018

Rename rendered_position to compute_layer_selection

Since "rendered position" is ambiguous, this patch renames it to
simply the function name implemented in the file.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I98d310b092edc0983ec03012f32c81f60224d2e0
Reviewed-on: https://chromium-review.googlesource.com/1160070
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580803}
[modify] https://crrev.com/440ed9790705720ebe88bdd76ba6ea1668424379/third_party/blink/renderer/core/editing/BUILD.gn
[rename] https://crrev.com/440ed9790705720ebe88bdd76ba6ea1668424379/third_party/blink/renderer/core/editing/compute_layer_selection.cc
[rename] https://crrev.com/440ed9790705720ebe88bdd76ba6ea1668424379/third_party/blink/renderer/core/editing/compute_layer_selection.h
[rename] https://crrev.com/440ed9790705720ebe88bdd76ba6ea1668424379/third_party/blink/renderer/core/editing/compute_layer_selection_test.cc
[modify] https://crrev.com/440ed9790705720ebe88bdd76ba6ea1668424379/third_party/blink/renderer/core/frame/local_frame_view.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 8

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

commit 1db7f209372b8413a2219f1166f5718b7f1be084
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Aug 08 16:23:23 2018

Remove unused function InlineTextBox::IsSelected.

Bug: 789870
Change-Id: I9f76d046c393645e148151660cab1f6510a874f0
Reviewed-on: https://chromium-review.googlesource.com/1166563
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581582}
[modify] https://crrev.com/1db7f209372b8413a2219f1166f5718b7f1be084/third_party/blink/renderer/core/layout/line/inline_text_box.cc
[modify] https://crrev.com/1db7f209372b8413a2219f1166f5718b7f1be084/third_party/blink/renderer/core/layout/line/inline_text_box.h

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 8

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

commit 9b1cc631171c9b50bfc7a66ed9851fccbaef29b7
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Aug 08 16:23:55 2018

Remove ellipsis SelectionState management.

We have not painted ellipsis selection:
https://chromium.googlesource.com/chromium/src/+/b066a99dbd93ff5ceed0f9488a2e9737138434d4
Then we don't need SelectionState codes around that.

Test: editing/selection/select-text-overflow-ellipsis*, paint/markers/ellipsis*
Bug: 789870
Change-Id: I646cb144258eb28a22e554eb3cbed14fc2043539
Reviewed-on: https://chromium-review.googlesource.com/1166755
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581583}
[modify] https://crrev.com/9b1cc631171c9b50bfc7a66ed9851fccbaef29b7/third_party/blink/renderer/core/layout/line/ellipsis_box.h
[modify] https://crrev.com/9b1cc631171c9b50bfc7a66ed9851fccbaef29b7/third_party/blink/renderer/core/layout/line/inline_text_box.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 16

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

commit 63eea6e3aa8520b01b08e170c171691518bfef4d
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Aug 16 04:21:08 2018

[LayoutNG] Use boolean instead of SelectionState for selection inline items.

SelectionState was used to represent selection in 2 ways:
1. If a LayoutObject is selected
2. If a InlineBox is selected.
This patch removes #2 usage by replacing it by boolean because almost implementation
only needs if it is selected or not.
Only exception is InlineTextBox.

The motivation is simplifying code that makes us more easily maintain it.

InlineTextBox::HasWrappedSelectionNewline()L240,L241:
 Since the InlineTextBox has the state SelectionState::kStart or kInside,
containing LayoutBlockFlow(= Root().Block()) have SelectionState::kContain.
Thus, this condition has been always true.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Idf4c5d4806c8ccef9eb2d9ce3c8f6dcf22891daf
Reviewed-on: https://chromium-review.googlesource.com/1168955
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583535}
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_box.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_box.h
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_flow_box.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_flow_box.h
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_text_box.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/inline_text_box.h
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/root_inline_box.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/layout/line/root_inline_box.h
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/paint/inline_text_box_painter.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/paint/svg_inline_text_box_painter.cc
[modify] https://crrev.com/63eea6e3aa8520b01b08e170c171691518bfef4d/third_party/blink/renderer/core/paint/svg_root_inline_box_painter.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 29

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

commit 374d0f6b15610eada09f473761de4615fdb64652
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Aug 29 12:28:15 2018

[LayoutNG] Unify selection start/end API into LayoutTextSelectionStatus.

|optional<int> FrameSelection::LayoutSelectionStart()/End()| is confusing API
because it returns the text offset where selection starts/end and the text
node could be different like |f^oo<br>ba|r| but the fact that selection
can wrap two or more nodes from the API is hard to catch and the API looked
rather "selection start/end offset on a node", which is completely wrong.
Plus, to get collect information, you should use with SelectionState.
We have such code here and there.

This patch unifies the API into
|LayoutTextSelectionStatus ComputeLayoutSelectionStatus(
    const LayoutText& text)|
so that users can get selected offset pair on the text w/o any confusion.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ib11cd358cc76d4464625a493803ea92c7032604e
Reviewed-on: https://chromium-review.googlesource.com/1179518
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587085}
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/editing/frame_selection.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/editing/frame_selection.h
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/editing/layout_selection.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/editing/layout_selection.h
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/editing/layout_selection_test.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/layout/api/line_layout_text.h
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/layout/layout_text.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/layout/layout_text_test.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/layout/line/inline_text_box.cc
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/layout/line/inline_text_box.h
[modify] https://crrev.com/374d0f6b15610eada09f473761de4615fdb64652/third_party/blink/renderer/core/page/touch_adjustment.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 6

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

commit 0931dc803165037b9abae0d567ec0a17ef2cf12b
Author: Yoichi Osato <yoichio@chromium.org>
Date: Thu Sep 06 01:10:01 2018

Add ComputeLayerSelectionTests for line break.

This patch adds some tests to confirm selection rect including
block end linebreaks.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ia0b30c333944c86d27bf6568e550645d60f5e319
Reviewed-on: https://chromium-review.googlesource.com/1205752
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589078}
[modify] https://crrev.com/0931dc803165037b9abae0d567ec0a17ef2cf12b/third_party/blink/renderer/core/editing/compute_layer_selection_test.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 11

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

commit 434458ed0436b62e2f395864f050bc752fd6d5a1
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Sep 11 04:26:02 2018

Fix line end caret w/o any following LayoutObject.

This patch fix ComputeInlineBoxPosition for the position that
points after line break and no LayoutObject after that by
fallback to previous position.
The fallback examples:
- "<br>|" to "|<br>"
- "<pre>foo\n|</pre>" to "<pre>foo|\n</pre>"

This change aligns legacy LocalCaretRect to NG.

Bug: 789870, 812535
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Icb6cd9b8bc58b56a04d01eb22bdd58bd16ad5a01
Reviewed-on: https://chromium-review.googlesource.com/1215506
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590199}
[modify] https://crrev.com/434458ed0436b62e2f395864f050bc752fd6d5a1/third_party/blink/renderer/core/editing/inline_box_position.cc
[modify] https://crrev.com/434458ed0436b62e2f395864f050bc752fd6d5a1/third_party/blink/renderer/core/editing/local_caret_rect_test.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 25

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

commit af7f7c7b7bb4c7c1f3d47f94cf7fef67e81e70d6
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Sep 25 07:55:56 2018

Add VisibleSelection canonicalization tests

This patch adds test cases for ComputeVisibleSelection
to record current behavior around BR, display=none and contenteditable.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I55ff0f446fcca178a0476d9686cd842d274fa369
Reviewed-on: https://chromium-review.googlesource.com/1237753
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593854}
[modify] https://crrev.com/af7f7c7b7bb4c7c1f3d47f94cf7fef67e81e70d6/third_party/blink/renderer/core/editing/visible_selection_test.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Sep 25

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

commit 2e2283bf1b81bd4dc8de2d922d00540905c55def
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Sep 25 08:34:14 2018

Unify current LayoutSelection info.

This patch refactors LayoutSelection members into a class which is
already used internally to compute the members.

This is a preparation for ComputeLayerSelection and
SelectionPaintRange.start/end_node members will be used to compute
layout selection.

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I6ef0154711e16b83eeb4889d9d3d41f545fb18fb
Reviewed-on: https://chromium-review.googlesource.com/1236619
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593861}
[modify] https://crrev.com/2e2283bf1b81bd4dc8de2d922d00540905c55def/third_party/blink/renderer/core/editing/layout_selection.cc
[modify] https://crrev.com/2e2283bf1b81bd4dc8de2d922d00540905c55def/third_party/blink/renderer/core/editing/layout_selection.h

Project Member

Comment 30 by bugdroid1@chromium.org, Oct 2

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

commit 0c221f1e0c33c608a2548959edf5ff97dc871cfc
Author: Yoichi Osato <yoichio@chromium.org>
Date: Tue Oct 02 01:34:18 2018

Create visible Selection w/o InlineBox for ComputeLayerSelection.

Design doc: http://bit.ly/2Njo0zR

This patch introduces |SelectionInDOMTree ComputeLayoutSelection
(const SelectionInDOMTree&)| for replacing CreateVisiblePosition
at ComputeLayerSelection.

The new adjustment algorithm is based on LayoutSelection and
really simple: pick SelectionState::kStart/kEnd nodes.
They are the start/end nodes of selection painting.

For caret, caret positioning should be done before LayoutSelection,
thus ComputeLayoutSelection() returns as-is.

Though ComputeLayoutSelection is targeting to replace
CreateVisibleSelection, there is difference because of simplicity:
it doesn't consider contenteditable, block-end BRs nor any InlinBox
matter. The difference is also tested at layout_selection_test.cc

Bug: 789870
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I9de9fef6e76ff65c1b2575c47d39e156d8e56380
Reviewed-on: https://chromium-review.googlesource.com/1182710
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595685}
[modify] https://crrev.com/0c221f1e0c33c608a2548959edf5ff97dc871cfc/third_party/blink/renderer/core/editing/compute_layer_selection.cc
[modify] https://crrev.com/0c221f1e0c33c608a2548959edf5ff97dc871cfc/third_party/blink/renderer/core/editing/frame_selection.h
[modify] https://crrev.com/0c221f1e0c33c608a2548959edf5ff97dc871cfc/third_party/blink/renderer/core/editing/layout_selection.cc
[modify] https://crrev.com/0c221f1e0c33c608a2548959edf5ff97dc871cfc/third_party/blink/renderer/core/editing/layout_selection.h
[modify] https://crrev.com/0c221f1e0c33c608a2548959edf5ff97dc871cfc/third_party/blink/renderer/core/editing/layout_selection_test.cc

Summary: Get rid of Frameselection::VisibleSelection from render pipeline (was: Create visible selection w/o InlineBox)
FrameSelection::ComputeVisibleSelectionInDOMTree is called from:
src/third_party/blink/renderer/core/editing/commands/clipboard_commands.cc
src/third_party/blink/renderer/core/editing/commands/delete_selection_command_test.cc
src/third_party/blink/renderer/core/editing/commands/editor_command.cc
src/third_party/blink/renderer/core/editing/commands/remove_format_command.cc
src/third_party/blink/renderer/core/editing/commands/style_commands.cc
src/third_party/blink/renderer/core/editing/commands/typing_command.cc
src/third_party/blink/renderer/core/editing/compute_layer_selection.cc
src/third_party/blink/renderer/core/editing/dom_selection.cc
src/third_party/blink/renderer/core/editing/editing_utilities.cc
src/third_party/blink/renderer/core/editing/editor.cc
src/third_party/blink/renderer/core/editing/finder/text_finder.cc
src/third_party/blink/renderer/core/editing/frame_selection.cc
src/third_party/blink/renderer/core/editing/frame_selection_test.cc
src/third_party/blink/renderer/core/editing/granularity_strategy.cc
src/third_party/blink/renderer/core/editing/ime/input_method_controller.cc
src/third_party/blink/renderer/core/editing/selection_controller.cc
src/third_party/blink/renderer/core/editing/selection_controller_test.cc
src/third_party/blink/renderer/core/editing/spellcheck/spell_checker.cc
src/third_party/blink/renderer/core/editing/suggestion/text_suggestion_controller.cc
src/third_party/blink/renderer/core/editing/visible_selection_test.cc
src/third_party/blink/renderer/core/exported/web_frame_test.cc
src/third_party/blink/renderer/core/exported/web_range.cc
src/third_party/blink/renderer/core/exported/web_surrounding_text.cc
src/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
src/third_party/blink/renderer/core/input/event_handler.cc
src/third_party/blink/renderer/core/input/mouse_event_manager.cc
src/third_party/blink/renderer/core/layout/layout_tree_as_text.cc
src/third_party/blink/renderer/core/page/drag_controller.cc
src/third_party/blink/renderer/core/svg/unsafe_svg_attribute_sanitization_test.cc

FrameSelection::ComputeVisibleSelectionInFlatTree is called from:
src/third_party/blink/renderer/core/editing/editor.cc
src/third_party/blink/renderer/core/editing/frame_selection.cc
src/third_party/blink/renderer/core/editing/frame_selection_test.cc
src/third_party/blink/renderer/core/editing/selection_controller.cc
src/third_party/blink/renderer/core/editing/suggestion/text_suggestion_controller.cc
src/third_party/blink/renderer/core/editing/suggestion/text_suggestion_controller_test.cc

Blockedon: 891169
Project Member

Comment 33 by bugdroid1@chromium.org, Oct 10

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

commit 1ffa88ecf5c8d650fc86075343b41faf08a605e4
Author: Yoichi Osato <yoichio@chromium.org>
Date: Wed Oct 10 04:27:16 2018

Revert "Create visible Selection w/o InlineBox for ComputeLayerSelection."

This reverts commit 0c221f1e0c33c608a2548959edf5ff97dc871cfc.

Reason for revert: Regression in Android.

Original change's description:
> Create visible Selection w/o InlineBox for ComputeLayerSelection.
> 
> Design doc: http://bit.ly/2Njo0zR
> 
> This patch introduces |SelectionInDOMTree ComputeLayoutSelection
> (const SelectionInDOMTree&)| for replacing CreateVisiblePosition
> at ComputeLayerSelection.
> 
> The new adjustment algorithm is based on LayoutSelection and
> really simple: pick SelectionState::kStart/kEnd nodes.
> They are the start/end nodes of selection painting.
> 
> For caret, caret positioning should be done before LayoutSelection,
> thus ComputeLayoutSelection() returns as-is.
> 
> Though ComputeLayoutSelection is targeting to replace
> CreateVisibleSelection, there is difference because of simplicity:
> it doesn't consider contenteditable, block-end BRs nor any InlinBox
> matter. The difference is also tested at layout_selection_test.cc
> 
> Bug: 789870
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Change-Id: I9de9fef6e76ff65c1b2575c47d39e156d8e56380
> Reviewed-on: https://chromium-review.googlesource.com/1182710
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Commit-Queue: Yoichi Osato <yoichio@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595685}

TBR=yosin@chromium.org,yoichio@chromium.org,xiaochengh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 789870, 892584
Change-Id: I8300194e28560b56a356586e7fc2939d3387c261
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1272239
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598211}
[modify] https://crrev.com/1ffa88ecf5c8d650fc86075343b41faf08a605e4/third_party/blink/renderer/core/editing/compute_layer_selection.cc
[modify] https://crrev.com/1ffa88ecf5c8d650fc86075343b41faf08a605e4/third_party/blink/renderer/core/editing/frame_selection.h
[modify] https://crrev.com/1ffa88ecf5c8d650fc86075343b41faf08a605e4/third_party/blink/renderer/core/editing/layout_selection.cc
[modify] https://crrev.com/1ffa88ecf5c8d650fc86075343b41faf08a605e4/third_party/blink/renderer/core/editing/layout_selection.h
[modify] https://crrev.com/1ffa88ecf5c8d650fc86075343b41faf08a605e4/third_party/blink/renderer/core/editing/layout_selection_test.cc

ComputeLayerSelection is delicate in that it computes start/end caret position of range selection.
That violates the selection assumption that selection is none, caret or range.
Since computing caret positions on range selection is rare case, we should
target more simple component for the first target of ComputeLayoutSelection.
Blocking: -707656 708452
Project Member

Comment 36 by bugdroid1@chromium.org, Nov 9

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

commit 1ce19971888cb7e52471ff5016ed34b21ab1e72e
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Nov 09 04:51:59 2018

Assert user selection start/end editability.

This patch introduces DCHECK in FrameSelection::SetSelectionDeprecated
so that user's selection start/end should have same editability for
code sanity.

Bug: 789870
Change-Id: Ia01192e61a9f27e2ddeef24e385b23a62a572174
Reviewed-on: https://chromium-review.googlesource.com/c/1325286
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606739}
[modify] https://crrev.com/1ce19971888cb7e52471ff5016ed34b21ab1e72e/third_party/blink/renderer/core/editing/frame_selection.cc

Comment 37 by yoichio@chromium.org, Yesterday (38 hours ago)

Labels: -Pri-1 Pri-2

Sign in to add a comment