[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor-browser] 207/311: Bug 1754459 - Improve caret position selection for images. r=dholbert a=dmeehan
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@xxxxxxxxx>
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/fa69d8b248e6c1df670aa6b019e30ec37e6672be/layout/generic/nsIFrame.cpp#4752
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>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits