Bookmarks WebUI: Add touch support for drag and drop in Bookmark Manager
This CLs differentiates when a drag event starts from a mouse vs touch,
and then signals the appropriate APIs. The front-end also now sends the
point at which the drag started so that the drag image can appear
immediately above the pointer, as opposed to the previous implementation
that revealed the drag image at the top left of the window. This is
especially important for when the drag starts with a long press touch
as the drag image indicates to the user that they can now drag the
bookmark.
We also now send GetContentNativeView instead of GetTopLevelNativeWindow
as that is where the touch event begins, allowing the GestureRecognizer
to properly transfer the touch_ids and transfer the consumer from the
Content's View to the drag and drop tracker. Without this, once the drag
event begins, the browser freezes and does not seem to recognize any
incoming gestures, touches, or other events.
Bug: 729861
Change-Id: If82eaac6ba32f98c2deb250721208aa353e1aebc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642273
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671064}
diff --git a/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc b/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc
index 90d56ba..c288b10 100644
--- a/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc
+++ b/chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api.cc
@@ -36,6 +36,7 @@
#include "components/undo/bookmark_undo_service.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "extensions/browser/extension_function_dispatcher.h"
@@ -411,12 +412,9 @@
source = ui::DragDropTypes::DRAG_EVENT_SOURCE_TOUCH;
chrome::DragBookmarks(GetProfile(),
- {
- std::move(nodes), params->drag_node_index,
- platform_util::GetViewForWindow(
- web_contents->GetTopLevelNativeWindow()),
- source,
- });
+ {std::move(nodes), params->drag_node_index,
+ web_contents->GetContentNativeView(), source,
+ gfx::Point(params->x, params->y)});
return true;
}
diff --git a/chrome/browser/resources/bookmarks/dnd_manager.js b/chrome/browser/resources/bookmarks/dnd_manager.js
index 0c9bf741..4db582fcd 100644
--- a/chrome/browser/resources/bookmarks/dnd_manager.js
+++ b/chrome/browser/resources/bookmarks/dnd_manager.js
@@ -330,6 +330,9 @@
* @private {!Object}
*/
this.timerProxy_ = window;
+
+ /** @private {boolean} */
+ this.lastPointerWasTouch_ = false;
}
init() {
@@ -344,7 +347,8 @@
'dragleave': this.onDragLeave_.bind(this),
'drop': this.onDrop_.bind(this),
'dragend': this.clearDragData_.bind(this),
- // TODO(calamity): Add touch support.
+ 'mousedown': this.onMouseDown_.bind(this),
+ 'touchstart': this.onTouchStart_.bind(this),
};
for (const event in this.documentListeners_) {
document.addEventListener(event, this.documentListeners_[event]);
@@ -407,9 +411,9 @@
const dragNodeIndex = draggedNodes.indexOf(dragElement.itemId);
assert(dragNodeIndex != -1);
- // TODO(calamity): account for touch.
chrome.bookmarkManagerPrivate.startDrag(
- draggedNodes, dragNodeIndex, false);
+ draggedNodes, dragNodeIndex, this.lastPointerWasTouch_, e.clientX,
+ e.clientY);
}
/** @private */
@@ -496,6 +500,16 @@
this.dropIndicator_.update(this.dropDestination_);
}
+ /** @private */
+ onMouseDown_() {
+ this.lastPointerWasTouch_ = false;
+ }
+
+ /** @private */
+ onTouchStart_() {
+ this.lastPointerWasTouch_ = true;
+ }
+
/**
* @private
* @param {DragData} dragData
diff --git a/chrome/browser/resources/bookmarks/item.js b/chrome/browser/resources/bookmarks/item.js
index 1b9ee64..e4d8147 100644
--- a/chrome/browser/resources/bookmarks/item.js
+++ b/chrome/browser/resources/bookmarks/item.js
@@ -37,6 +37,9 @@
/** @private */
isFolder_: Boolean,
+
+ /** @private */
+ lastTouchPoints_: Number,
},
hostAttributes: {
@@ -55,6 +58,7 @@
'auxclick': 'onMiddleClick_',
'mousedown': 'cancelMiddleMouseBehavior_',
'mouseup': 'cancelMiddleMouseBehavior_',
+ 'touchstart': 'onTouchStart_',
},
/** @override */
@@ -82,6 +86,14 @@
onContextMenu_: function(e) {
e.preventDefault();
e.stopPropagation();
+
+ // Prevent context menu from appearing after a drag, but allow opening the
+ // context menu through 2 taps
+ if (e.sourceCapabilities && e.sourceCapabilities.firesTouchEvents &&
+ this.lastTouchPoints_ !== 2) {
+ return;
+ }
+
this.focus();
if (!this.isSelectedItem_) {
this.selectThisItem_();
@@ -210,6 +222,14 @@
},
/**
+ * @param {TouchEvent} e
+ * @private
+ */
+ onTouchStart_: function(e) {
+ this.lastTouchPoints_ = e.touches.length;
+ },
+
+ /**
* Prevent default middle-mouse behavior. On Windows, this prevents autoscroll
* (during mousedown), and on Linux this prevents paste (during mouseup).
* @param {MouseEvent} e
diff --git a/chrome/browser/ui/bookmarks/bookmark_browsertest.cc b/chrome/browser/ui/bookmarks/bookmark_browsertest.cc
index 71ab269..c265dce 100644
--- a/chrome/browser/ui/bookmarks/bookmark_browsertest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_browsertest.cc
@@ -225,13 +225,15 @@
const GURL page_url("http://www.google.com");
const BookmarkNode* root = model->bookmark_bar_node();
const BookmarkNode* node = model->AddURL(root, 0, page_title, page_url);
+ const gfx::Point expected_point(100, 100);
auto run_loop = std::make_unique<base::RunLoop>();
chrome::DoBookmarkDragCallback cb = base::BindLambdaForTesting(
- [&run_loop, page_title, page_url](
+ [&run_loop, page_title, page_url, expected_point](
const ui::OSExchangeData& drag_data, gfx::NativeView native_view,
- ui::DragDropTypes::DragEventSource source, int operation) {
+ ui::DragDropTypes::DragEventSource source, gfx::Point point,
+ int operation) {
GURL url;
base::string16 title;
EXPECT_TRUE(drag_data.provider().GetURLAndTitle(
@@ -246,6 +248,7 @@
// See https://crbug.com/893388.
EXPECT_FALSE(drag_data.provider().GetDragImage().isNull());
#endif
+ EXPECT_EQ(expected_point, point);
run_loop->Quit();
});
@@ -255,7 +258,8 @@
{{node},
kDragNodeIndex,
platform_util::GetViewForWindow(browser()->window()->GetNativeWindow()),
- ui::DragDropTypes::DRAG_EVENT_SOURCE_MOUSE},
+ ui::DragDropTypes::DRAG_EVENT_SOURCE_MOUSE,
+ expected_point},
std::move(cb));
run_loop->Run();
@@ -270,13 +274,15 @@
const BookmarkNode* root = model->bookmark_bar_node();
const BookmarkNode* node1 = model->AddURL(root, 0, page_title, page_url);
const BookmarkNode* node2 = model->AddFolder(root, 0, page_title);
+ const gfx::Point expected_point(100, 100);
auto run_loop = std::make_unique<base::RunLoop>();
chrome::DoBookmarkDragCallback cb = base::BindLambdaForTesting(
- [&run_loop](const ui::OSExchangeData& drag_data,
- gfx::NativeView native_view,
- ui::DragDropTypes::DragEventSource source, int operation) {
+ [&run_loop, expected_point](const ui::OSExchangeData& drag_data,
+ gfx::NativeView native_view,
+ ui::DragDropTypes::DragEventSource source,
+ gfx::Point point, int operation) {
#if !defined(OS_MACOSX)
GURL url;
base::string16 title;
@@ -293,6 +299,7 @@
// See https://crbug.com/893388.
EXPECT_FALSE(drag_data.provider().GetDragImage().isNull());
#endif
+ EXPECT_EQ(expected_point, point);
run_loop->Quit();
});
@@ -304,6 +311,7 @@
platform_util::GetViewForWindow(
browser()->window()->GetNativeWindow()),
ui::DragDropTypes::DRAG_EVENT_SOURCE_MOUSE,
+ expected_point,
},
std::move(cb));
diff --git a/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc b/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc
index 41ff1fe..5b3983e29 100644
--- a/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_drag_drop.cc
@@ -28,11 +28,13 @@
std::vector<const bookmarks::BookmarkNode*> nodes,
int drag_node_index,
gfx::NativeView view,
- ui::DragDropTypes::DragEventSource source)
+ ui::DragDropTypes::DragEventSource source,
+ gfx::Point start_point)
: nodes(std::move(nodes)),
drag_node_index(drag_node_index),
view(view),
- source(source) {}
+ source(source),
+ start_point(start_point) {}
BookmarkDragParams::~BookmarkDragParams() = default;
int DropBookmarks(Profile* profile,
diff --git a/chrome/browser/ui/bookmarks/bookmark_drag_drop.h b/chrome/browser/ui/bookmarks/bookmark_drag_drop.h
index 48b9b6c..71c1f94 100644
--- a/chrome/browser/ui/bookmarks/bookmark_drag_drop.h
+++ b/chrome/browser/ui/bookmarks/bookmark_drag_drop.h
@@ -9,6 +9,7 @@
#include "build/build_config.h"
#include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/gfx/geometry/point.h"
#include "ui/gfx/native_widget_types.h"
class Profile;
@@ -30,13 +31,15 @@
base::OnceCallback<void(const ui::OSExchangeData& drag_data,
gfx::NativeView native_view,
ui::DragDropTypes::DragEventSource source,
+ gfx::Point start_point,
int operation)>;
struct BookmarkDragParams {
BookmarkDragParams(std::vector<const bookmarks::BookmarkNode*> nodes,
int drag_node_index,
gfx::NativeView view,
- ui::DragDropTypes::DragEventSource source);
+ ui::DragDropTypes::DragEventSource source,
+ gfx::Point start_point);
~BookmarkDragParams();
// The bookmark nodes to be dragged.
@@ -50,6 +53,9 @@
// The source of the drag.
ui::DragDropTypes::DragEventSource source;
+
+ // The point the drag started.
+ gfx::Point start_point;
};
// Starts the process of dragging a folder of bookmarks.
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc b/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc
index b51cff7..a1554c41 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_drag_drop_views.cc
@@ -174,6 +174,7 @@
count_(params.nodes.size()),
native_view_(params.view),
source_(params.source),
+ start_point_(params.start_point),
do_drag_callback_(std::move(do_drag_callback)),
observer_(this),
weak_factory_(this) {
@@ -229,7 +230,7 @@
BookmarkDragImageSource::kDragImageOffsetY));
std::move(do_drag_callback_)
- .Run(drag_data_, native_view_, source_, operation_);
+ .Run(drag_data_, native_view_, source_, start_point_, operation_);
delete this;
}
@@ -260,6 +261,7 @@
int count_;
gfx::NativeView native_view_;
ui::DragDropTypes::DragEventSource source_;
+ const gfx::Point start_point_;
int operation_;
DoBookmarkDragCallback do_drag_callback_;
@@ -277,13 +279,17 @@
void DoDragImpl(const ui::OSExchangeData& drag_data,
gfx::NativeView native_view,
ui::DragDropTypes::DragEventSource source,
+ gfx::Point point,
int operation) {
// Allow nested run loop so we get DnD events as we drag this around.
base::MessageLoopCurrent::ScopedNestableTaskAllower nestable_task_allower;
views::Widget* widget = views::Widget::GetWidgetForNativeView(native_view);
- DCHECK(widget);
- widget->RunShellDrag(nullptr, drag_data, gfx::Point(), operation, source);
+ if (widget) {
+ widget->RunShellDrag(nullptr, drag_data, gfx::Point(), operation, source);
+ } else {
+ views::RunShellDrag(native_view, drag_data, point, operation, source);
+ }
}
void DragBookmarksImpl(Profile* profile,
diff --git a/chrome/common/extensions/api/bookmark_manager_private.json b/chrome/common/extensions/api/bookmark_manager_private.json
index 922e930c..19bfd70 100644
--- a/chrome/common/extensions/api/bookmark_manager_private.json
+++ b/chrome/common/extensions/api/bookmark_manager_private.json
@@ -123,6 +123,18 @@
"name": "isFromTouch",
"type": "boolean",
"description": "True if the drag was initiated from touch."
+ },
+ {
+ "name": "x",
+ "type": "integer",
+ "minimum": 0,
+ "description": "The clientX of the dragStart event"
+ },
+ {
+ "name": "y",
+ "type": "integer",
+ "minimum": 0,
+ "description": "The clientY of the dragStart event"
}
]
},
diff --git a/third_party/closure_compiler/externs/bookmark_manager_private.js b/third_party/closure_compiler/externs/bookmark_manager_private.js
index eb74345..3089f776 100644
--- a/third_party/closure_compiler/externs/bookmark_manager_private.js
+++ b/third_party/closure_compiler/externs/bookmark_manager_private.js
@@ -74,8 +74,10 @@
* @param {!Array<string>} idList An array of string-valued ids.
* @param {number} dragNodeIndex The index of the dragged node in |idList|
* @param {boolean} isFromTouch True if the drag was initiated from touch.
+ * @param {number} offsetX The offset X of the event
+ * @param {number} offsetY The offset Y of the event
*/
-chrome.bookmarkManagerPrivate.startDrag = function(idList, dragNodeIndex, isFromTouch) {};
+chrome.bookmarkManagerPrivate.startDrag = function(idList, dragNodeIndex, isFromTouch, offsetX, offsetY) {};
/**
* Performs the drop action of the drag and drop session.
