This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1 in repository tor-browser.
commit 8bc2b1176fa7044585d5e29cb0f18b39ad4b75ba Author: Emilio Cobos Álvarez emilio@crisal.io AuthorDate: Wed Mar 16 23:32:44 2022 +0000
Bug 1754459 - Improve caret position selection for images. r=dholbert a=dmeehan
What saves us in the non-pointer-events: none case is silly, and is this loop that hits the draggable image:
https://searchfox.org/mozilla-central/rev/fa69d8b248e6c1df670aa6b019e30ec37e...
Needs tests (if it doesn't affect existing ones), but uploading for some initial feedback. I think this makes a lot more sense than the existing code, and works for editing and dragging the same way, so it's nice to have less special-cases.
Differential Revision: https://phabricator.services.mozilla.com/D141242 --- layout/generic/nsIFrame.cpp | 43 +++++++++++++++++----- layout/generic/test/mochitest.ini | 1 + layout/generic/test/test_image_selection_3.html | 48 +++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 9 deletions(-)
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp index c4fc8ac39f81f..be46eba2110bc 100644 --- a/layout/generic/nsIFrame.cpp +++ b/layout/generic/nsIFrame.cpp @@ -5638,21 +5638,45 @@ static FrameTarget GetSelectionClosestFrameForBlock(nsIFrame* aFrame, return DrillDownToSelectionFrame(aFrame, true, aFlags); }
-// Use frame edge for grid, flex, table, and non-draggable & non-editable -// image frames. +// Use frame edge for grid, flex, table, and non-editable images. Choose the +// edge based on the point position past the frame rect. If past the middle, +// caret should be at end, otherwise at start. This behavior matches Blink. +// +// TODO(emilio): Can we use this code path for other replaced elements other +// than images? Or even all other frames? We only get there when we didn't find +// selectable children... At least one XUL test fails if we make this apply to +// XUL labels. Also, editable images need _not_ to use the frame edge, see +// below. static bool UseFrameEdge(nsIFrame* aFrame) { if (aFrame->IsFlexOrGridContainer() || aFrame->IsTableFrame()) { return true; } - if (static_cast<nsImageFrame*>(do_QueryFrame(aFrame))) { - return !nsContentUtils::ContentIsDraggable(aFrame->GetContent()) && - !aFrame->GetContent()->IsEditable(); + const nsImageFrame* image = do_QueryFrame(aFrame); + if (image && !aFrame->GetContent()->IsEditable()) { + // Editable images are a special-case because editing relies on clicking on + // an editable image selecting it, for it to show resizers. + return true; } return false; }
-static FrameTarget LastResortFrameTargetForFrame(nsIFrame* aFrame) { - return {aFrame, UseFrameEdge(aFrame), false}; +static FrameTarget LastResortFrameTargetForFrame(nsIFrame* aFrame, + const nsPoint& aPoint) { + if (!UseFrameEdge(aFrame)) { + return {aFrame, false, false}; + } + const auto& rect = aFrame->GetRectRelativeToSelf(); + nscoord reference; + nscoord middle; + if (aFrame->GetWritingMode().IsVertical()) { + reference = aPoint.y; + middle = rect.Height() / 2; + } else { + reference = aPoint.x; + middle = rect.Width() / 2; + } + const bool afterFrame = reference > middle; + return {aFrame, true, afterFrame}; }
// GetSelectionClosestFrame is the helper function that calculates the closest @@ -5661,7 +5685,8 @@ static FrameTarget LastResortFrameTargetForFrame(nsIFrame* aFrame) { // restricted environments. // Cannot handle overlapping frames correctly, so it should receive the output // of GetFrameForPoint -// Guaranteed to return a valid FrameTarget +// Guaranteed to return a valid FrameTarget. +// aPoint is relative to aFrame. static FrameTarget GetSelectionClosestFrame(nsIFrame* aFrame, const nsPoint& aPoint, uint32_t aFlags) { @@ -5685,7 +5710,7 @@ static FrameTarget GetSelectionClosestFrame(nsIFrame* aFrame, } }
- return LastResortFrameTargetForFrame(aFrame); + return LastResortFrameTargetForFrame(aFrame, aPoint); }
static nsIFrame::ContentOffsets OffsetsForSingleFrame(nsIFrame* aFrame, diff --git a/layout/generic/test/mochitest.ini b/layout/generic/test/mochitest.ini index 08b1e08a81a20..c640340ce87c2 100644 --- a/layout/generic/test/mochitest.ini +++ b/layout/generic/test/mochitest.ini @@ -108,6 +108,7 @@ support-files = frame_visibility_in_iframe_child.html [test_image_selection.html] [test_image_selection_2.html] +[test_image_selection_3.html] [test_image_selection_in_contenteditable.html] [test_intrinsic_size_on_loading.html] [test_movement_by_characters.html] diff --git a/layout/generic/test/test_image_selection_3.html b/layout/generic/test/test_image_selection_3.html new file mode 100644 index 0000000000000..32a49b54d0bec --- /dev/null +++ b/layout/generic/test/test_image_selection_3.html @@ -0,0 +1,48 @@ +<!doctype html> +<title>Test for bug 1754459</title> +<script src="/tests/SimpleTest/SimpleTest.js"></script> +<script src="/tests/SimpleTest/EventUtils.js"></script> +<link rel="stylesheet" href="/tests/SimpleTest/test.css"/> +<style> + /* This avoids the draggable image check that our code does to avoid handling the mousedown. */ + img { pointer-events: none } +</style> +<div id="block"> + Some text <img width="100" height="100" id="image" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAyAQMAAACQ%2B%2Bz9AAAAA1BMVEUA%2FwA0XsCoAAAAD0lEQVQoFWNgGAWjYGgCAAK8AAEb3eOQAAAAAElFTkSuQmCC"> and more. +</div> +<script> +const image = document.getElementById("image"); +const block = document.getElementById("block"); +const selection = window.getSelection(); + +function clickOnImage(x, y) { + synthesizeMouse(image, x, y, {}); +} + +add_task(async function test_click_pointer_events_none() { + await SimpleTest.promiseFocus(window); + clickOnImage(10, 10); + ok(selection.isCollapsed, "Should be collapsed"); + is(selection.focusNode, block, "Should be at block"); + is(selection.focusOffset, 1, "Should be at start of image"); + + clickOnImage(60, 10); + ok(selection.isCollapsed, "Should be collapsed"); + is(selection.focusNode, block, "Should be at block"); + is(selection.focusOffset, 2, "Should be at end of image"); +}); + +add_task(async function test_click_pointer_events_none_vertical() { + block.style.writingMode = "vertical-lr"; + block.getBoundingClientRect(); + clickOnImage(10, 10); + ok(selection.isCollapsed, "Should be collapsed"); + is(selection.focusNode, block, "Should be at start of image"); + is(selection.focusOffset, 1, "Should be at start of image"); + + clickOnImage(10, 60); + ok(selection.isCollapsed, "Should be collapsed"); + is(selection.focusNode, block, "Should be at block"); + is(selection.focusOffset, 2, "Should be at end of image"); +}); +</script>