New issue
Advanced search Search tips

Issue 794282 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Change calls of PositionTemplate::IsOrphan/IsConnected to IsValidFor(document)

Project Member Reported by xiaoche...@chromium.org, Dec 12 2017

Issue description

PositionTemplate::IsOrphan::IsConnected() checks whether the anchor node is connected in the DOM/Flat tree. Most callers use them to check if a position is (unexpectedly) moved out of document, and if so, add extra handling.

However, these callers of them actually need to check more. When the position is moved to another document, we are unable to detect such case by IsOrphan() or IsConnected().

These callers should check IsValidFor(document) instead to also check whether the position is still in the same document.

Note:

1. These functions handle null positions in slightly different ways:
   - IsConnected(): returns false
   - IsOrphan(): returns false
   - IsValidFor(document): returns true regardless of |document|
   So the conversion might not be purely mechanical, depending on whether the callers expect null positions.

2. Callers of VisiblePositionTemplate::IsOrphan() should also be changed
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 18

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

commit c052c5a049eaa722d54834213ea3b235d06c157a
Author: tanvir.rizvi <tanvir.rizvi@samsung.com>
Date: Thu Oct 18 06:09:50 2018

Use IsValidFor instead of IsConnected in DeleteSelectionCommand

IsConnected checks if the anchor node is connected in DOM/FLAT
tree, while IsValidFor check if the position is in the same
document.
HandleGeneralDelete can cause DOM mutation and hence
can move out of the document. Therefore we should use
IsValidFor.
This CL changes IsConnected to IsValidFor to cover
all the scenarios.

Bug: 794282
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I72268bfe1e275e40768aaf7f8e875e20b99f6460
Reviewed-on: https://chromium-review.googlesource.com/c/1286242
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#600672}
[modify] https://crrev.com/c052c5a049eaa722d54834213ea3b235d06c157a/third_party/blink/renderer/core/editing/commands/delete_selection_command.cc

Sign in to add a comment