Caret painting is broken inside multicol
Reported by
msten...@opera.com,
Mar 18 2016
|
||
Issue descriptionCaret painting is broken when displaying the caret inside a direct child of a multicol container, in any column but the first. The reason for the bug is that we don't agree with ourselves where the boundary for flowthread vs. visual coordinates goes. In LayoutObject, the mapLocalToAncestor() / offsetFromContainer() machinery converts from flow thread coordinates to visual coordinates between flow thread children and the flow thread when walking the tree upwards. The mapToVisibleRectInAncestorSpace() machinery, on the other hand, performs this conversion between the flow thread and its parent multicol container (i.e. one level further up in the tree). So we risk performing the conversion twice, if we switch machineries on our way - which the CaretBase code in editing happens to do. The latter (the mapToVisibleRectInAncestorSpace() way) is conceptually more correct (it's when you escape the flow thread that you're no longer in the flow thread coordinate space), but in any case, all parts of the engine should at least do it at the same point in the layout tree.
,
Mar 18 2016
We get focus ring rects wrong, as well, huh? :-/
,
Mar 18 2016
Aye. Same reason.
,
Mar 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8796ca1186b3ff3e3da105b3ee41f1698be01821 commit 8796ca1186b3ff3e3da105b3ee41f1698be01821 Author: mstensho <mstensho@opera.com> Date: Mon Mar 21 18:43:17 2016 Shift flowthread-to-visual coordinate space conversion one level up in the tree. The conversion now takes place between the flow thread and its parent multicol container, rather than between the flow thread and its children. This is both conceptually more correct, and it also matches what mapToVisibleRectInAncestorSpace() already does. Having all machineries do this at the same place in the tree is what fixes the editing-specific bug 596070 . As for layer clipping bug 527709 , it just so happens that we specify the flow thread as ancestor in mapLocalToAncestor(), which is invoked via localToAncestorPoint() from PaintLayerClipper::calculateClipRects(). PaintLayerClipper does its work *before* fragments have been collected and set up for a given layer, so it doesn't want mapLocalToAncestor() or anyone to change to the visual coordinate space. BUG= 527709 , 596070 R=leviw@chromium.org Review URL: https://codereview.chromium.org/1819603003 Cr-Commit-Position: refs/heads/master@{#382339} [add] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/LayoutTests/editing/caret/in-multicol-child-expected.html [add] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/LayoutTests/editing/caret/in-multicol-child.html [add] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/LayoutTests/fast/multicol/abspos-in-overflow-hidden-in-2nd-column-expected.html [add] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/LayoutTests/fast/multicol/abspos-in-overflow-hidden-in-2nd-column.html [modify] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/Source/core/editing/CaretBase.cpp [modify] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp [modify] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/Source/core/layout/LayoutObject.cpp [modify] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/Source/core/layout/MapCoordinatesTest.cpp [modify] https://crrev.com/8796ca1186b3ff3e3da105b3ee41f1698be01821/third_party/WebKit/Source/core/layout/api/LayoutItem.h
,
Mar 21 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by msten...@opera.com
, Mar 18 2016