New issue
Advanced search Search tips

Issue 826478 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

MacViewsBrowser: "Use Selection for Find" does not work

Project Member Reported by rsesek@chromium.org, Mar 27 2018

Issue description

Chrome Version: 67.0.3381.0
OS: macOS 10.13.3

What steps will reproduce the problem?
(1) --enable-features=ViewsBrowserWindows
(2) Go to http://dev.chromium.org/
(3) Highlight a word like "Chromium" on the page
(4) Press Cmd+E or go to Edit > Find > Use Selection for Find
(5) Press Cmd+G or go to Edit > Find > Find Next

What is the expected result?
The selected word from step (3) is loaded into the find pasteboard, and selecting Find Next in step (5) opens Find in Page with that selected word.

What happens instead?
The find pasteboard is not updated in step (4).

Please use labels and text to provide additional information.


For graphics-related bugs, please copy/paste the contents of the about:gpu
page at the end of this report.

 
Labels: M-68 Target-68
Owner: robliao@chromium.org
Status: Assigned (was: Untriaged)
MacViews triage: to robliao@ for M68.

Comment 2 by rsesek@chromium.org, Mar 29 2018

 Issue 827189  may be related.

Comment 3 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Labels: Sprint-1
Status: Started (was: Assigned)
This is likely due to a missing equivalent registration:
https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.mm?rcl=d453881f0d6c5c37e4be5241899729bdde0e1d13&l=89
[[NSNotificationCenter defaultCenter]
        addObserver:self
           selector:@selector(findPboardUpdated:)
               name:kFindPasteboardChangedNotification
             object:[FindPasteboard sharedInstance]];

Comment 7 by gov...@chromium.org, Apr 25 2018

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
Labels: Sprint-2
Cc: ellyjo...@chromium.org
Can we remove "Sprint-1" label as "Sprint-2" is already applied at #8?
Labels: -Sprint-1
Removing "Sprint-1" label per https://bugs.chromium.org/p/chromium/issues/detail?id=712244#c11.
Any progress here?
This should be in code review soon.
Thank you for the update robliao@.
Project Member

Comment 14 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/626e8f4e769a669f0060451d720b28a4a10310d9

commit 626e8f4e769a669f0060451d720b28a4a10310d9
Author: Robert Liao <robliao@chromium.org>
Date: Thu May 10 22:56:09 2018

Refactor the Cocoa Pasteboard Integration Up to the Chrome Find Bar Level

This allows Cocoa and Views to use the same pasteboard notification.
This does not hook up views Find Bar updates to the pasteboard.

BUG= 826478 , 827189 

Change-Id: I038fe4b3acb9e3884ca928d9cbc5106a1a324913
Reviewed-on: https://chromium-review.googlesource.com/1053548
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557716}
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.mm
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_controller.h
[add] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_platform_helper.cc
[add] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_platform_helper.h
[add] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_platform_helper_mac.mm
[add] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/find_bar/find_bar_platform_helper_mac_browsertest.mm
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/views/find_bar_host.cc
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/browser/ui/views/find_bar_views_interactive_uitest.cc
[modify] https://crrev.com/626e8f4e769a669f0060451d720b28a4a10310d9/chrome/test/BUILD.gn

Can this be marked as fixed if nothing else is pending?
Status: Fixed (was: Started)
Project Member

Comment 17 by bugdroid1@chromium.org, May 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3

commit 2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3
Author: Robert Liao <robliao@chromium.org>
Date: Fri May 11 16:46:06 2018

Propagate the Views Find Bar Text Changes to the Pasteboard

Adds support to get the views find bar text to the global pasteboard.
Also fixes a Cocoa bug introduced in the previous pasteboard change
where the selection is always reset to the beginning at every keystroke.

BUG= 826478 , 827189 , 842131 

Change-Id: If1a639f1997e3fea7fee2c66959e7dcdb2fd3bc7
Reviewed-on: https://chromium-review.googlesource.com/1055195
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557902}
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/find_bar/find_bar_controller.cc
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/find_bar/find_bar_controller.h
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/find_bar/find_bar_platform_helper.h
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/find_bar/find_bar_platform_helper_mac.mm
[add] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/find_bar/find_bar_platform_helper_mac_interactive_uitest.mm
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/browser/ui/views/find_bar_view.cc
[modify] https://crrev.com/2a741a1fd691061cb7ff8c93a4a6d9a1fa5937c3/chrome/test/BUILD.gn

Sign in to add a comment