Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(219)

Issue 2855353002: Make Paste Popup use selection rect for positioning (Closed)

Created:
3 years, 7 months ago by amaralp
Modified:
3 years, 7 months ago
CC:
agrieve+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, jam, kinuko+watch, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, nona+watch_chromium.org, rlanday, shuchen+watch_chromium.org, James Su, yusukes+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make Paste Popup use selection rect for positioning Currently the paste popup and the selection action mode are being positioned differently. This CL makes it so the paste popup is positioned in the same manner as the selection action mode. BUG=627234 Review-Url: https://codereview.chromium.org/2855353002 Cr-Commit-Position: refs/heads/master@{#471433} Committed: https://chromium.googlesource.com/chromium/src/+/354e86bea96bc19c3d922f3733b81a8f8b635993

Patch Set 1 #

Total comments: 11

Patch Set 2 : Addressing comments #

Patch Set 3 : Fix typo #

Total comments: 3

Patch Set 4 : Calculate selection rect better #

Total comments: 1

Patch Set 5 : Adding handle height to selection rect #

Patch Set 6 : fix typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -89 lines) Patch
M content/browser/android/content_view_core_impl.cc View 1 2 3 4 1 chunk +6 lines, -3 lines 0 comments Download
M content/browser/android/selection_popup_controller.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/android/selection_popup_controller.cc View 2 chunks +2 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 3 4 5 2 chunks +8 lines, -2 lines 0 comments Download
M content/common/frame_messages.h View 1 chunk +1 line, -4 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 3 1 chunk +4 lines, -2 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java View 1 2 3 8 chunks +19 lines, -18 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/FloatingPastePopupMenu.java View 1 4 chunks +4 lines, -32 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/LegacyPastePopupMenu.java View 2 chunks +3 lines, -2 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/PastePopupMenu.java View 2 chunks +4 lines, -2 lines 0 comments Download
M content/public/common/context_menu_params.h View 2 chunks +4 lines, -11 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 1 chunk +3 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/web/ContextMenuClientImpl.cpp View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
M third_party/WebKit/public/web/WebContextMenuData.h View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download

Messages

Total messages: 39 (26 generated)
amaralp
PTAL, decided to split this this CL off the larger crrev.com/2785853002. boliu@ for content/browser and ...
3 years, 7 months ago (2017-05-04 18:41:23 UTC) #7
Avi (use Gerrit)
context_menu_params.h lgtm
3 years, 7 months ago (2017-05-04 18:44:01 UTC) #8
boliu
https://codereview.chromium.org/2855353002/diff/1/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java File content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java (right): https://codereview.chromium.org/2855353002/diff/1/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java#newcode96 content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java:96: // Selection rectangle in DIP. no longer in DIP? ...
3 years, 7 months ago (2017-05-04 20:36:11 UTC) #9
aelias_OOO_until_Jul13
https://codereview.chromium.org/2855353002/diff/1/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp File third_party/WebKit/Source/web/ContextMenuClientImpl.cpp (right): https://codereview.chromium.org/2855353002/diff/1/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#newcode409 third_party/WebKit/Source/web/ContextMenuClientImpl.cpp:409: anchor_webrect.x + anchor_webrect.width, C++ rects have different format from ...
3 years, 7 months ago (2017-05-04 20:48:56 UTC) #10
amaralp
https://codereview.chromium.org/2855353002/diff/1/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java File content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java (right): https://codereview.chromium.org/2855353002/diff/1/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java#newcode96 content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java:96: // Selection rectangle in DIP. On 2017/05/04 at 20:36:11, ...
3 years, 7 months ago (2017-05-05 01:14:32 UTC) #19
boliu
lgtm
3 years, 7 months ago (2017-05-05 02:03:10 UTC) #20
dcheng
ipc lgtm
3 years, 7 months ago (2017-05-05 06:08:12 UTC) #21
aelias_OOO_until_Jul13
https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp File third_party/WebKit/Source/web/ContextMenuClientImpl.cpp (right): https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#newcode407 third_party/WebKit/Source/web/ContextMenuClientImpl.cpp:407: web_view_->SelectionBounds(focus_webrect, anchor_webrect); The rect you're computing here would be ...
3 years, 7 months ago (2017-05-06 01:18:39 UTC) #22
amaralp
https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp File third_party/WebKit/Source/web/ContextMenuClientImpl.cpp (right): https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#newcode407 third_party/WebKit/Source/web/ContextMenuClientImpl.cpp:407: web_view_->SelectionBounds(focus_webrect, anchor_webrect); On 2017/05/06 at 01:18:39, aelias wrote: > ...
3 years, 7 months ago (2017-05-11 18:46:55 UTC) #27
aelias_OOO_until_Jul13
https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp File third_party/WebKit/Source/web/ContextMenuClientImpl.cpp (right): https://codereview.chromium.org/2855353002/diff/40001/third_party/WebKit/Source/web/ContextMenuClientImpl.cpp#newcode407 third_party/WebKit/Source/web/ContextMenuClientImpl.cpp:407: web_view_->SelectionBounds(focus_webrect, anchor_webrect); On 2017/05/11 at 18:46:55, amaralp wrote: > ...
3 years, 7 months ago (2017-05-11 21:16:17 UTC) #28
aelias_OOO_until_Jul13
OK, after some offline discussion we realized changing the rect unnecessarily sends us down a ...
3 years, 7 months ago (2017-05-11 22:08:44 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2855353002/100001
3 years, 7 months ago (2017-05-12 18:36:46 UTC) #36
commit-bot: I haz the power
3 years, 7 months ago (2017-05-12 20:37:22 UTC) #39
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/354e86bea96bc19c3d922f3733b8...

Powered by Google App Engine
This is Rietveld 408576698