New issue
Advanced search Search tips

Issue 834066 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Wrong hittest result for images in RTL blocks

Project Member Reported by xiaoche...@chromium.org, Apr 17 2018

Issue description

Chrome Version: 65.0.3325.181 (Official Build) (64-bit)
OS: Linux

What steps will reproduce the problem?
(1) Navigate to https://jsfiddle.net/htvbyxxs/3/
(2) Drag mouse from leftmost to rightmost, or
(3) Drag mouse from rightmost to leftmost

What is the expected result?

After both (2) and (3), all three images are selected

What happens instead?

Can't select all three images. Selection flickers.

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 16 2018

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

commit f95316ca7de9727cbaa873de8220b85dbec10983
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Wed May 16 03:53:22 2018

Fix hit testing on replaced elements with RTL resolved direction

When hit-testing in a replaced element, existing code hard maps its left
half to the DOM position before the element, and right half to the position
after the element, which is wrong if the element is in an RTL bidi run.

This patch introduced LayoutBox::ResolvedDirection() to obtain the direction
of the bidi run that an atomic inline box belongs to, and applies it to fix
the hit-testing behavior on replaced elements in bidi text.

Note: this patch also fixed the behavior in LayoutNG, as LayoutNG currently
shares LayoutReplaced::PositionForPoint() with legacy.

Bug: 811502,  834066 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I1800f9a655c2330e4ff4dda048246e3f6d62a798
Reviewed-on: https://chromium-review.googlesource.com/1056029
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558964}
[modify] https://crrev.com/f95316ca7de9727cbaa873de8220b85dbec10983/third_party/WebKit/LayoutTests/editing/assert_selection.js
[add] https://crrev.com/f95316ca7de9727cbaa873de8220b85dbec10983/third_party/WebKit/LayoutTests/editing/selection/mouse/drag_selects_bidi_image.html
[modify] https://crrev.com/f95316ca7de9727cbaa873de8220b85dbec10983/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/f95316ca7de9727cbaa873de8220b85dbec10983/third_party/blink/renderer/core/layout/layout_box.h
[modify] https://crrev.com/f95316ca7de9727cbaa873de8220b85dbec10983/third_party/blink/renderer/core/layout/layout_replaced.cc

Project Member

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

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

commit 9a125e84b03f3e05299103f85ca0ae1f7fb7f3f4
Author: Xiaocheng Hu <xiaochengh@chromium.org>
Date: Thu May 17 03:32:40 2018

Fix hit testing outside RTL atomic inlines

LayoutBlock::PositionForPointIfOutsideAtomicInlineLevel() hard-maps
the left side of an atomic inline to the DOM position at its start, and
the right side to its end, which is wrong if the atomic inline is RTL.

This patch fixes the issue.

Note: this patch doesn't fix selection flickering when dragging mouse
over an atomic inline surrounded by bidi text, due to bugs in bidi
adjustment in SelectionController.

Bug: 811502,  834066 
Change-Id: Id1687922f3d713cb2eb4b8668cb0687196a2fc44
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/1062831
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559413}
[add] https://crrev.com/9a125e84b03f3e05299103f85ca0ae1f7fb7f3f4/third_party/WebKit/LayoutTests/editing/selection/mouse/drag_selects_rtl_inline_block.html
[modify] https://crrev.com/9a125e84b03f3e05299103f85ca0ae1f7fb7f3f4/third_party/blink/renderer/core/layout/layout_block.cc

Status: Fixed (was: Available)
Hit test result is now correct.

However, mouse dragging still creates flickering selections in some cases, due to (many) bug in bidi adjustment: https://jsfiddle.net/sd7r8mk8/7/

Sign in to add a comment