Selection jumps when crossing float's boundaries |
|||||
Issue descriptionChrome Version: 60.0.3112.78 (Official Build) (64-bit) OS: linux What steps will reproduce the problem? (1) Load the attached test case (2) Start selecting inside the float box (3) Extend selection outside the float box What is the expected result? Highlighted text should match start/end visual selection point. What happens instead? The selection start/end points are inverted, so the highlighted text is the one from the beginning of the float box to the selection start point.
,
Aug 25 2017
,
Aug 25 2017
I'm working on a patch that may solve, or at least improve, the behavior of selection with floats. It seems that we deliberately ignore float elements when evaluating HitTest candidates. We indeed do it explicitly in LayoutBlock::IsChildHitTestCandidate when processing the position of a selection point for block elements and implicitly when dealing with inline elements in LayoutBlockFlow::PositionForPoint function. My current approach is to stop ignoring float elements and try to figure out the closest candidate to the selection point. Is this a valid approach or are there good reasons to continue ignoring floats ?
,
Aug 25 2017
,
Aug 29 2017
I recently worked on (and reviewed) some hit testing code and found it a difficult area because it's poorly spec'd (e.g., hit testing is sort-of paint order, selection is sort-of dom-order, and paint order can be completely independent of dom order: https://goo.gl/2wfE7x). My intuition is to err on the side of cross-browser compatibility. Safari/WebKit, Firefox/Gecko, and Blink all have the jumping behavior in your example, and only Edge has behavior you're suggesting. Would be possible to get together with folks from WebKit and Gecko (and Edge too) to see if this could be made in all engines at once?
,
Aug 29 2017
Regarding floats: did you look at the code revision history to see why floats were excluded in the first place? Hopefully there are attached use cases that could be studied as part of this bug.
,
Aug 29 2017
My idea is to work in parallel on WebKit (https://webkit.org/b/10177 so hopefully we'll reach and agreement about the expected behavior. We should only land patches which don't break browser interoperability. I'll file a bug on Firefox to gather additional opinions.
,
Aug 29 2017
I think a 1 was dropped; the correct url for WebKit is https://webkit.org/b/101771.
,
Sep 6 2017
Filed a similar bug in FF https://bugzilla.mozilla.org/show_bug.cgi?id=1397514
,
Sep 6 2017
Yes, sorry about the WK bug mistake. We can consider https://webkit.org/b/101771 as the meta-bug for all this selection issues with floats. I've filed https://bugs.webkit.org/show_bug.cgi?id=176096 as a first step. That is what my patch in https://crrev.com/c/643106 addresses now.
,
Sep 12 2017
FTR, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1397514 for Firefox to track this issue there.
,
Oct 5 2017
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebf3a5282adcb4eaa862516508517d57ae052009 commit ebf3a5282adcb4eaa862516508517d57ae052009 Author: Javier Fernandez <jfernandez@igalia.com> Date: Tue Dec 12 12:06:05 2017 Consider floats as HitTest candidate for selection Selection with floats has a weird and unpredictable behavior. The root cause is that floats are not considered valid HitTest candidates. There are many other cases where float elements are ignored when traversing the tree looking for the closes element to a specific LayoutPoint. This patch address just one of this cases, where floats are children of a in-flow block-level box. Bug: 758526 Change-Id: I918af3ee21aa1070fa03a9fe1073205cc0a57300 Reviewed-on: https://chromium-review.googlesource.com/643106 Reviewed-by: Philip Rogers (OOO) <pdr@chromium.org> Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Cr-Commit-Position: refs/heads/master@{#523410} [add] https://crrev.com/ebf3a5282adcb4eaa862516508517d57ae052009/third_party/WebKit/LayoutTests/editing/selection/select-out-of-floated-non-editable.html [modify] https://crrev.com/ebf3a5282adcb4eaa862516508517d57ae052009/third_party/WebKit/Source/core/layout/LayoutBlock.cpp
,
Nov 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b726621f5d8dd271e0b5c238d3770245b6ab6047 commit b726621f5d8dd271e0b5c238d3770245b6ab6047 Author: Javier Fernandez <jfernandez@igalia.com> Date: Mon Nov 19 12:24:17 2018 Revert "Consider floats as HitTest candidate for selection" This reverts commit ebf3a5282adcb4eaa862516508517d57ae052009. Reason for revert: This change caused a regression in how caret is positioned in contenteditable areas with floats or multicolumns. Original change's description: > Consider floats as HitTest candidate for selection > > Selection with floats has a weird and unpredictable behavior. The root > cause is that floats are not considered valid HitTest candidates. > > There are many other cases where float elements are ignored when > traversing the tree looking for the closes element to a specific > LayoutPoint. > > This patch address just one of this cases, where floats are children > of a in-flow block-level box. > > Bug: 758526 > Change-Id: I918af3ee21aa1070fa03a9fe1073205cc0a57300 > Reviewed-on: https://chromium-review.googlesource.com/643106 > Reviewed-by: Philip Rogers (OOO) <pdr@chromium.org> > Commit-Queue: Javier Fernandez <jfernandez@igalia.com> > Cr-Commit-Position: refs/heads/master@{#523410} TBR=yosin@chromium.org,pdr@chromium.org,jfernandez@igalia.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 758526, 888424 Change-Id: Iba0f1641eeae63a52ef914b334cb302e69b7e7d9 Reviewed-on: https://chromium-review.googlesource.com/c/1271102 Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Javier Fernandez <jfernandez@igalia.com> Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Cr-Commit-Position: refs/heads/master@{#609248} [delete] https://crrev.com/9ca81adb5affc813264c45c26b98cfcc96576a14/third_party/WebKit/LayoutTests/editing/selection/select-out-of-floated-non-editable.html [modify] https://crrev.com/b726621f5d8dd271e0b5c238d3770245b6ab6047/third_party/blink/renderer/core/layout/layout_block.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by jfernan...@igalia.com
, Aug 24 2017