Update Omnibox and Suggestions Popup to Material Refresh Look |
|||||||||||||||||||||||
Issue descriptionTracks engineering work to update the Omnibox and its associated suggestions popup to the new Material Refresh spec.
,
Mar 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/092c628a6330f273eedb2607ffe318c7e0951db0 commit 092c628a6330f273eedb2607ffe318c7e0951db0 Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Mar 29 20:32:15 2018 UI: Move Shadow from wm::Shadow to ui::Shadow To implement the Material Refresh UI, we need to draw this elevation-based shadow on all platforms, not just ones that we use the Window manager on. Therefore we are moving this Shadow class from ui/wm to ui/compositor. Forked off of: https://chromium-review.googlesource.com/c/chromium/src/+/869699. Bug: 823535 Change-Id: Ib12ca299ab15512022228283d946c8403751525a Reviewed-on: https://chromium-review.googlesource.com/978590 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#546923} [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/BUILD.gn [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/wm/overview/window_grid.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/wm/overview/window_grid.h [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/wm/overview/window_selector_item.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/wm/overview/window_selector_item.h [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ash/wm/workspace/phantom_window_controller.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/chrome/browser/ui/ash/chrome_keyboard_ui.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/chrome/browser/ui/ash/chrome_keyboard_ui.h [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/components/exo/BUILD.gn [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/components/exo/client_controlled_shell_surface_unittest.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/components/exo/shell_surface_base.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/aura/BUILD.gn [add] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/compositor_extra/BUILD.gn [add] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/compositor_extra/DEPS [rename] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/compositor_extra/shadow.cc [rename] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/compositor_extra/shadow.h [rename] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/compositor_extra/shadow_unittest.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/wm/BUILD.gn [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/wm/core/DEPS [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/wm/core/shadow_controller.cc [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/wm/core/shadow_controller.h [modify] https://crrev.com/092c628a6330f273eedb2607ffe318c7e0951db0/ui/wm/core/shadow_controller_unittest.cc
,
Apr 11 2018
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2352ee8dacf0838d5453223a5e043d35d27861ec commit 2352ee8dacf0838d5453223a5e043d35d27861ec Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Apr 20 22:26:10 2018 Omnibox Material Refresh: Update background color to match Refresh spec Updates the background color of Omnibox in Material Refresh. Only affects behavior when Touchable or MD Refresh flag is on. Bug: 823535 Change-Id: I2bf12f72011d78855edc51207f078128d9181aed Reviewed-on: https://chromium-review.googlesource.com/1019735 Reviewed-by: Dave Schuyler <dschuyler@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#552494} [modify] https://crrev.com/2352ee8dacf0838d5453223a5e043d35d27861ec/chrome/browser/ui/omnibox/omnibox_theme.cc
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a996afc1165b8923319a2ce21278abf78cf57e4f commit a996afc1165b8923319a2ce21278abf78cf57e4f Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Apr 24 01:01:51 2018 Omnibox Material Refresh: Search Provider Favicon Scaffolding For Material Refresh, we want to place the default search provider's favicon into the location bar while the user is typing search queries. This CL adds some scaffolding necessary for OmniboxView to provide the favicon. The actual favicon fetching must be asynchronous, so we need this extra infrastructure. I expect this CL to be Part 1/4 or so of the primary implementation. Bug: 823535 Change-Id: Ia7d6979222fd18e4a34fd1473f00164ecc8a1c7b Reviewed-on: https://chromium-review.googlesource.com/1022500 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#552933} [modify] https://crrev.com/a996afc1165b8923319a2ce21278abf78cf57e4f/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm [modify] https://crrev.com/a996afc1165b8923319a2ce21278abf78cf57e4f/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/a996afc1165b8923319a2ce21278abf78cf57e4f/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/a996afc1165b8923319a2ce21278abf78cf57e4f/components/omnibox/browser/omnibox_view.cc [modify] https://crrev.com/a996afc1165b8923319a2ce21278abf78cf57e4f/components/omnibox/browser/omnibox_view.h
,
Apr 25 2018
,
Apr 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03fc3da46d200ae95171d1379c960414b73c79ec commit 03fc3da46d200ae95171d1379c960414b73c79ec Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Apr 25 16:11:08 2018 Omnibox Material Refresh: Search Provider Favicons from History DB This CL hooks up the OmniboxView to the OmniboxClient, which retrieves the favicon for the default search provider from the FaviconService. The implementation below is a good start but there needs to be further work in these two areas: a) Our coverage is currently limited to what FaviconService can provide us, but that is not adequate. Most search engine favicons are never populated into the FaviconService database. See bug 88243. b) We need an in-memory synchronous cache to avoid hits to the sqlite Favicons database on every keystroke. This CL works for search engines with a populated favicon. For example, visiting search.yahoo.com, and then switching the default search provider to Yahoo will work. Bug: 823535 Change-Id: Ie4e4425d9cfa59f971bc3264c1b79104d1e5d958 Reviewed-on: https://chromium-review.googlesource.com/1026695 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#553590} [modify] https://crrev.com/03fc3da46d200ae95171d1379c960414b73c79ec/chrome/browser/ui/omnibox/chrome_omnibox_client.cc [modify] https://crrev.com/03fc3da46d200ae95171d1379c960414b73c79ec/chrome/browser/ui/omnibox/chrome_omnibox_client.h [modify] https://crrev.com/03fc3da46d200ae95171d1379c960414b73c79ec/components/omnibox/browser/omnibox_client.cc [modify] https://crrev.com/03fc3da46d200ae95171d1379c960414b73c79ec/components/omnibox/browser/omnibox_client.h [modify] https://crrev.com/03fc3da46d200ae95171d1379c960414b73c79ec/components/omnibox/browser/omnibox_view.cc
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b67bd278e394b14f9941223111a058c258777e2d commit b67bd278e394b14f9941223111a058c258777e2d Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 02 00:59:08 2018 Views on Windows: Force NativeWidgetAura for shadow_elevation Widgets On Windows, Widgets with shadow_elevation specified are currently drawn incorrectly. This is because we spawn a native top-level window, which ignores shadow_elevation. This is to be expected, since the Windows window manager has no concept of Material shadow elevation. This CL forces those Widgets with shadow_elevation to use an Aura non-toplevel window. The caveat is that these widgets are now clipped to the root browser window. (It can't be bigger.) But it has the benefits of being: 1. Low maintainence 2. Click-through shadow 3. Proper hit testing Bug: 823535 , 838667 Change-Id: I008ab64d0f3b7c2811d11ac94f00d041c45bf730 Reviewed-on: https://chromium-review.googlesource.com/1028833 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#555267} [modify] https://crrev.com/b67bd278e394b14f9941223111a058c258777e2d/chrome/browser/ui/views/chrome_views_delegate_win.cc
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9edc43ab6dd02e95a4535d3624c8c2146dcdef1e commit 9edc43ab6dd02e95a4535d3624c8c2146dcdef1e Author: Tommy C. Li <tommycli@chromium.org> Date: Mon May 07 21:30:00 2018 Search Engines: Update default search provider favicons Updates the Google and Bing search provider favicons. Bug: 88243 , 823535 Change-Id: Icd8ffa664058ce5b117dc1ed38c589de55b145fc Reviewed-on: https://chromium-review.googlesource.com/1043354 Commit-Queue: Peter Kasting <pkasting@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#556574} [modify] https://crrev.com/9edc43ab6dd02e95a4535d3624c8c2146dcdef1e/components/search_engines/prepopulated_engines.json
,
May 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c663cfa38efc6fd48e5610d84f83ad68328c40f commit 9c663cfa38efc6fd48e5610d84f83ad68328c40f Author: Tommy C. Li <tommycli@chromium.org> Date: Tue May 08 18:52:36 2018 Omnibox: Move FaviconCache to the Omnibox component. We move FaviconCache to the Omnibox component for two reasons: 1) Since there's no need for this to live in chrome/, it shouldn't. We generally favor code living in components/ for modularity. 2) Favicon component OWNERs have indicated that this cache may one day be included into the Favicon component if there is another user other than Omnibox. Bug: 823535 Change-Id: I5f4464f868f069ab6d5fa2ab9cb6e94b84eec7e6 Reviewed-on: https://chromium-review.googlesource.com/1048409 Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#556902} [modify] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/chrome/browser/ui/omnibox/chrome_omnibox_client.h [modify] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/chrome/test/BUILD.gn [modify] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/components/omnibox/browser/BUILD.gn [modify] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/components/omnibox/browser/DEPS [rename] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/components/omnibox/browser/favicon_cache.cc [rename] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/components/omnibox/browser/favicon_cache.h [rename] https://crrev.com/9c663cfa38efc6fd48e5610d84f83ad68328c40f/components/omnibox/browser/favicon_cache_unittest.cc
,
May 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d93a90a94939dd7105393a5d3587ad0b354fc23 commit 8d93a90a94939dd7105393a5d3587ad0b354fc23 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 09 19:54:41 2018 Omnibox Material Refresh: Reuse FaviconCache for Search Provider favicon This CL extends FaviconCache to be able to handle favicons fetched by an icon's URL. It previously could only handle fetching favicons by a page's URL. We use this newly extended FaviconCache as a synchronous cache for ChromeOmniboxClient's default search provider favicon fetching. Note one caveat: Previously ChromeOmniboxClient would drop all but the most recent request for the default search provider favicon. That optimization was dropped in this CL, but I now think it was a premature optimization anyways. Bug: 823535 Change-Id: I01760a8937d1f0eb99548599006091409f749dd9 Reviewed-on: https://chromium-review.googlesource.com/1048973 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#557287} [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/chrome/browser/ui/omnibox/chrome_omnibox_client.cc [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/chrome/browser/ui/omnibox/chrome_omnibox_client.h [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/components/omnibox/browser/favicon_cache.cc [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/components/omnibox/browser/favicon_cache.h [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/components/omnibox/browser/favicon_cache_unittest.cc [modify] https://crrev.com/8d93a90a94939dd7105393a5d3587ad0b354fc23/components/omnibox/browser/omnibox_client.h
,
May 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a32858b4e80133c6f56a5f8bd50d60d5145d5a0 commit 8a32858b4e80133c6f56a5f8bd50d60d5145d5a0 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 09 22:55:50 2018 Omnibox Material Refresh: Clean LocationBarView::OnLocationIconFetched Clean up an unnecessary call within: LocationBarView::OnLocationIconFetched. Previously we also called LocationIconView::Update, which is unnecessary. Bug: 823535 Change-Id: I87e9a83ef2d831ead9d674c3ea7f98bd06830cf4 Reviewed-on: https://chromium-review.googlesource.com/1052917 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#557360} [modify] https://crrev.com/8a32858b4e80133c6f56a5f8bd50d60d5145d5a0/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a25d9e175ee4127c5c21fdc210be13833cf3b2a commit 4a25d9e175ee4127c5c21fdc210be13833cf3b2a Author: Tommy C. Li <tommycli@chromium.org> Date: Thu May 10 00:00:32 2018 Search Engines: Increment version number for prepopulated_engines.json As a followup to: https://chromium-review.googlesource.com/c/chromium/src/+/1043354 Also increment the data version number so clients get the new data. Bug: 88243 , 823535 Change-Id: I77f67fd8d5d435526bee4f0c9c2773f2fc927d51 Reviewed-on: https://chromium-review.googlesource.com/1053288 Reviewed-by: Peter Kasting <pkasting@chromium.org> Commit-Queue: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#557381} [modify] https://crrev.com/4a25d9e175ee4127c5c21fdc210be13833cf3b2a/components/search_engines/prepopulated_engines.json
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2589a7a54bc600ce3880cf827cff59a72f1a9aba commit 2589a7a54bc600ce3880cf827cff59a72f1a9aba Author: Tommy C. Li <tommycli@chromium.org> Date: Fri May 11 00:44:31 2018 Search Engines: Update the search provider favicons at runtime Currently, the search providers all have static prepopulated favicon URLs. These can fall out of date. This CL updates the search providers' favicons as the user browses. Specifically, when the user actually lands on a URL matching the a search provider URL template, and we get a favicon that does not match the stored favicon URL, we update the stored favicon URL. Bug: 88243 , 823535 Change-Id: I43f72ad449515d5a19e5ec4b32ca996b9d32eb11 Reviewed-on: https://chromium-review.googlesource.com/1048174 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#557742} [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/chrome/browser/ui/search_engines/search_engine_tab_helper.cc [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/chrome/browser/ui/search_engines/search_engine_tab_helper.h [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/components/search_engines/default_search_manager.cc [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/components/search_engines/template_url.cc [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/components/search_engines/template_url_service.cc [modify] https://crrev.com/2589a7a54bc600ce3880cf827cff59a72f1a9aba/components/search_engines/template_url_service.h
,
May 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/21e8d8c8dc11cd4b8f77c47b7f37098ebde13d3a commit 21e8d8c8dc11cd4b8f77c47b7f37098ebde13d3a Author: Tommy C. Li <tommycli@chromium.org> Date: Sat May 12 00:18:10 2018 HistoryBackend: NotifyFaviconsChanged on icon creation as well Previously, we only notified NotifyFaviconsChanged when an existing icon was modified, not on initial creation. The FaviconCache in the Omnibox component caches null results from the FaviconService, so we want to be notified when a new favicon is downloaded. This supports the Chrome Material Refresh work. There are only two users of this API so far, the Bookmarks, and Sync. For bookmarks, notifying when a new icon is downloaded should be harmless. Bookmarks should already have a favicon in the database, so it generally shouldn't happen. Even if it does happen, it would cause some "extra work" to occur as the bookmark icon is updated - nothing catastrophic, and potentially beneficial (maybe we should have been doing this work on favicon creation anyways). The Sync observer ignores icon_url updates already. Bug: 88243 , 823535 Change-Id: Ia233f1dfa83d7e748c43b4f0117f56b14424a1a8 Reviewed-on: https://chromium-review.googlesource.com/1054568 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#558068} [modify] https://crrev.com/21e8d8c8dc11cd4b8f77c47b7f37098ebde13d3a/components/history/core/browser/history_backend.cc [modify] https://crrev.com/21e8d8c8dc11cd4b8f77c47b7f37098ebde13d3a/components/history/core/browser/history_backend_unittest.cc
,
May 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/798c6eda2c3d169d9e6e16203b00d484227a487e commit 798c6eda2c3d169d9e6e16203b00d484227a487e Author: Tommy C. Li <tommycli@chromium.org> Date: Tue May 15 19:57:52 2018 Omnibox: Update FaviconCache to observe favicon changed notifications Previously we were timing out 'null' favicon results after 60 seconds. This has turned out to be too simple of a heuristic now that we are using the FaviconCache to back the default search provider icon in the location bar. Users would expect that the favicon appear immediately after loading a matching search results page, and would be confused by a 60 second delay. Bug: 88243 , 823535 Change-Id: I062361ffad0e0bdaa45f44ad67a7071efa086f2c Reviewed-on: https://chromium-review.googlesource.com/1055787 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#558816} [modify] https://crrev.com/798c6eda2c3d169d9e6e16203b00d484227a487e/components/omnibox/browser/favicon_cache.cc [modify] https://crrev.com/798c6eda2c3d169d9e6e16203b00d484227a487e/components/omnibox/browser/favicon_cache.h [modify] https://crrev.com/798c6eda2c3d169d9e6e16203b00d484227a487e/components/omnibox/browser/favicon_cache_unittest.cc
,
May 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9b9470fbfb74491bd5831af98ae2e18b429ac21 commit c9b9470fbfb74491bd5831af98ae2e18b429ac21 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 16 22:22:15 2018 Omnibox UI Refresh: Update background color to white when popup open Previously, it was only white in the touchable modes, but it should be white when the popup is open in all the newer Material modes. Bug: 837500, 823535 Change-Id: Ifd52903798553f50645148a9d44c6a80ee0eb479 Reviewed-on: https://chromium-review.googlesource.com/1062504 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#559310} [modify] https://crrev.com/c9b9470fbfb74491bd5831af98ae2e18b429ac21/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/362b262e6353f7d4db40b0923f2ca7bee949895a commit 362b262e6353f7d4db40b0923f2ca7bee949895a Author: Tommy C. Li <tommycli@chromium.org> Date: Thu May 17 18:40:12 2018 Omnibox UI Refresh: Make background color white on focus In Touchable, the background color of the Omnibox is usually gray, but set to white when the popup is open. In Material Refresh, we go even further. We set the background color to white whenever the Omnibox is focused. Bug: 837500, 823535 Change-Id: Ib52a5819eb0727878389cf0fd5ce482b8a0a30aa Reviewed-on: https://chromium-review.googlesource.com/1062810 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#559620} [modify] https://crrev.com/362b262e6353f7d4db40b0923f2ca7bee949895a/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/362b262e6353f7d4db40b0923f2ca7bee949895a/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc
,
May 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/169388e6f4eec7db5886a4e33c6ecac8d3880a3b commit 169388e6f4eec7db5886a4e33c6ecac8d3880a3b Author: Tommy C. Li <tommycli@chromium.org> Date: Fri May 18 16:42:05 2018 Omnibox UI: Clean up OmniboxClient API a bit. Remove an unused parameter and some no-op IOS overrides. This is a CL to clean up the API in preparation for potentially changing the meaning of IsPopupOpen for the Material Refresh UI. Bug: 823535 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I10638a3bc873448f49961c705157e541257adfa4 Reviewed-on: https://chromium-review.googlesource.com/1065022 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#559919} [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/chrome/browser/ui/omnibox/chrome_omnibox_client.cc [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/chrome/browser/ui/omnibox/chrome_omnibox_client.h [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/components/omnibox/browser/omnibox_client.h [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/components/omnibox/browser/omnibox_edit_model.cc [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h [modify] https://crrev.com/169388e6f4eec7db5886a4e33c6ecac8d3880a3b/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm
,
May 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7d36327724e88b545db24c0e7002407d77a8b0f commit f7d36327724e88b545db24c0e7002407d77a8b0f Author: Tommy C. Li <tommycli@chromium.org> Date: Mon May 21 21:56:52 2018 Omnibox UI Refresh: Implement Elevated Popup for Clobber State In the current Omnibox, we only showed the elevated popup when there are Autocomplete results to display. In the new UI Material Refresh, we display the elevated frame (with no results) whenever the Omnibox is focused and user input is in progress. This CL implements this. TBR=rdevlin.cronin@chromium.org Bug: 823535 Change-Id: Ia1bf5597123a4bf576bc5d9ef714d760560fcf0e Reviewed-on: https://chromium-review.googlesource.com/1066457 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#560362} [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/extensions/api/omnibox/omnibox_api_interactive_test.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac_unittest.mm [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/search/search_tab_helper.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_controller.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_edit_model.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_edit_model.h [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_edit_model_unittest.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_popup_model.cc [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_popup_model.h [modify] https://crrev.com/f7d36327724e88b545db24c0e7002407d77a8b0f/components/omnibox/browser/omnibox_popup_view.h
,
May 21 2018
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a3f9e3c891650110375813315343c5be03d2dadc commit a3f9e3c891650110375813315343c5be03d2dadc Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 30 17:11:58 2018 Omnibox UI Refresh: Update Secure chip color to gray for Refresh When the Refresh flag is on, after this CL, even secure sites will appear as gray. Also to match the mocks, incognito mode icons have been recolored to Google Gray 200 instead of 100 when Refresh flag is on. Bug: 841542 , 823535 Change-Id: Iaa0febad225a5ac7437a6614e82f6345ccc1860d Reviewed-on: https://chromium-review.googlesource.com/1072904 Reviewed-by: Peter Boström <pbos@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#562878} [modify] https://crrev.com/a3f9e3c891650110375813315343c5be03d2dadc/chrome/browser/ui/omnibox/omnibox_theme.cc
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be652b796af75662f2d0dc64e152d8c90b14f271 commit be652b796af75662f2d0dc64e152d8c90b14f271 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed May 30 20:30:01 2018 Omnibox UI Refresh: Always show the focus ring when focused Previously, we only showed the focus ring on certain platforms (Mac) or keyboard accessibility mode. Now, following the new Material Refresh guidelines, we display the focus ring on the Omnibox whenever it is focused (and the popup is hidden). This also simplifies the code a bit, as keyboard accessibility mode is no longer relevant. Bug: 844263 , 823535 Change-Id: Ic5133463f6e67af5ef6f0512034938e4a19a9972 Reviewed-on: https://chromium-review.googlesource.com/1069690 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#562979} [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/chrome/browser/ui/views/toolbar/toolbar_view.h [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/ui/views/controls/focus_ring.cc [modify] https://crrev.com/be652b796af75662f2d0dc64e152d8c90b14f271/ui/views/controls/focus_ring.h
,
Jun 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9c838db3d55e12815963062e220fd097024cdbc commit c9c838db3d55e12815963062e220fd097024cdbc Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Jun 01 00:02:56 2018 Revert "Omnibox UI Refresh: Implement Elevated Popup for Clobber State" This reverts commit f7d36327724e88b545db24c0e7002407d77a8b0f. Reason for revert: UX decided to back off of this change for Refresh. Original change's description: > Omnibox UI Refresh: Implement Elevated Popup for Clobber State > > In the current Omnibox, we only showed the elevated popup when there > are Autocomplete results to display. > > In the new UI Material Refresh, we display the elevated frame (with no > results) whenever the Omnibox is focused and user input is in progress. > > This CL implements this. > > TBR=rdevlin.cronin@chromium.org > > Bug: 823535 > Change-Id: Ia1bf5597123a4bf576bc5d9ef714d760560fcf0e > Reviewed-on: https://chromium-review.googlesource.com/1066457 > Commit-Queue: Tommy Li <tommycli@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Cr-Commit-Position: refs/heads/master@{#560362} TBR=pkasting@chromium.org,tommycli@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 823535 Change-Id: I35b99f6193ff6eb0d06c154e3d9839798bf0e385 Reviewed-on: https://chromium-review.googlesource.com/1081074 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#563446} [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/extensions/api/omnibox/omnibox_api_interactive_test.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac_unittest.mm [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/search/search_tab_helper.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_controller.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_edit_model.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_edit_model.h [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_edit_model_unittest.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_popup_model.cc [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_popup_model.h [modify] https://crrev.com/c9c838db3d55e12815963062e220fd097024cdbc/components/omnibox/browser/omnibox_popup_view.h
,
Jun 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/479e0af586ef7a0b0770312a9f03a13aa72910c2 commit 479e0af586ef7a0b0770312a9f03a13aa72910c2 Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Jun 01 21:50:31 2018 Omnibox UI Refresh: Fix legacy focus ring for keyboard accessibility Previously we displayed a legacy focus ring if Material Refresh was off for full keyboard accessibilty mode on all platforms. The CL below accidentally removed it: https://chromium-review.googlesource.com/c/chromium/src/+/1069690 Now we are adding it back with a partial revert - and a refactor to make the logic overall more clear. We can remove everything we are adding in this CL once Material Refresh is permanent and launched. Bug: 848641 , 844263 , 823535 Change-Id: Ife78cd9660e38f16525b55044ee0a231101534a6 Reviewed-on: https://chromium-review.googlesource.com/1082794 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#563814} [modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/479e0af586ef7a0b0770312a9f03a13aa72910c2/chrome/browser/ui/views/toolbar/toolbar_view.h
,
Jun 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/075283400818b1c283dcf74543411d9d9c1d9c1e commit 075283400818b1c283dcf74543411d9d9c1d9c1e Author: Tommy C. Li <tommycli@chromium.org> Date: Sat Jun 02 00:45:28 2018 Omnibox UI Refresh: Show placeholder "Search Google..." text in Omnibox Shows "Search Google or type a web address" in the Omnibox when Google is the default search provider - just like in the NTP Fakebox. Bug: 823535 Change-Id: I8c3b130b5840d89a092e5bb7b73179d56d128288 Reviewed-on: https://chromium-review.googlesource.com/1083615 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#563885} [modify] https://crrev.com/075283400818b1c283dcf74543411d9d9c1d9c1e/chrome/app/generated_resources.grd [modify] https://crrev.com/075283400818b1c283dcf74543411d9d9c1d9c1e/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/075283400818b1c283dcf74543411d9d9c1d9c1e/chrome/browser/ui/views/omnibox/omnibox_view_views.h [modify] https://crrev.com/075283400818b1c283dcf74543411d9d9c1d9c1e/ui/views/controls/textfield/textfield.cc [modify] https://crrev.com/075283400818b1c283dcf74543411d9d9c1d9c1e/ui/views/controls/textfield/textfield.h
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cab33d1317a67d2512b8677a91611b5b5d18d91 commit 5cab33d1317a67d2512b8677a91611b5b5d18d91 Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Jun 04 23:38:23 2018 Omnibox UI Refresh: Implement popup open and close opacity animations. This eases in and out the Omnibox popup opacity over 82ms as it opens and closes. This matches the animation spec provided by UX. Bug: 823535 Change-Id: Ic2add1f57f63ae6d043812f2bd5a766f84d302c9 Reviewed-on: https://chromium-review.googlesource.com/1081198 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Ali Juma <ajuma@chromium.org> Cr-Commit-Position: refs/heads/master@{#564295} [modify] https://crrev.com/5cab33d1317a67d2512b8677a91611b5b5d18d91/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/5cab33d1317a67d2512b8677a91611b5b5d18d91/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc [modify] https://crrev.com/5cab33d1317a67d2512b8677a91611b5b5d18d91/ui/compositor/closure_animation_observer.cc [modify] https://crrev.com/5cab33d1317a67d2512b8677a91611b5b5d18d91/ui/compositor/closure_animation_observer.h
,
Jun 5 2018
,
Jun 5 2018
I'd like to merge the revert here: https://bugs.chromium.org/p/chromium/issues/detail?id=823535#c24 Revert "Omnibox UI Refresh: Implement Elevated Popup for Clobber State" This reverts commit f7d36327724e88b545db24c0e7002407d77a8b0f. Reason for revert: UX decided to back off of this change for Refresh. Original change's description: > Omnibox UI Refresh: Implement Elevated Popup for Clobber State > > In the current Omnibox, we only showed the elevated popup when there > are Autocomplete results to display. > > In the new UI Material Refresh, we display the elevated frame (with no > results) whenever the Omnibox is focused and user input is in progress. > > This CL implements this. > > TBR=rdevlin.cronin@chromium.org > > Bug: 823535 > Change-Id: Ia1bf5597123a4bf576bc5d9ef714d760560fcf0e > Reviewed-on: https://chromium-review.googlesource.com/1066457 > Commit-Queue: Tommy Li <tommycli@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Cr-Commit-Position: refs/heads/master@{#560362} TBR=pkasting@chromium.org,tommycli@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 823535 Change-Id: I35b99f6193ff6eb0d06c154e3d9839798bf0e385 Reviewed-on: https://chromium-review.googlesource.com/1081074 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#563446} To M68.
,
Jun 5 2018
This bug requires manual review: There is .grd file changes and we are only 48 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba1f800d50e758cd057765bec1b16019b615be19 commit ba1f800d50e758cd057765bec1b16019b615be19 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jun 06 15:10:32 2018 Update Fakebox and Omnibox to display "Search Google or type a URL". Previously we had updated it to say: "Search Google or type a web address" That was not correct. We are sticking with "a URL". Bug: 823535 Change-Id: I99f2c1d1397136001ef81b2dbaa4d2059babf35b Reviewed-on: https://chromium-review.googlesource.com/1087846 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#564893} [modify] https://crrev.com/ba1f800d50e758cd057765bec1b16019b615be19/chrome/app/generated_resources.grd
,
Jun 6 2018
Has this landed in canary yet? Seems like it was reverted.
,
Jun 6 2018
Approving #24 for merge to M68. Branch:3440
,
Jun 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e07aadc746e5d902fdd314fbb2ba0f692a58e43 commit 1e07aadc746e5d902fdd314fbb2ba0f692a58e43 Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Jun 08 20:09:02 2018 Omnibox Refresh: Update Icons to 32x24 rounded rect shape Previously they were 24x24. But in the new Material Refresh spec, they take up 32x24. Bug: 849779 , 823535 Change-Id: Ie1f5dfe9e9f2ae88bcf07643fb6b33f6472bdcdf Reviewed-on: https://chromium-review.googlesource.com/1090073 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Peter Boström <pbos@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#565723} [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/layout_constants.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/layout_constants.h [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/location_bar/keyword_hint_view.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/omnibox/omnibox_match_cell_view.cc [modify] https://crrev.com/1e07aadc746e5d902fdd314fbb2ba0f692a58e43/chrome/browser/ui/views/page_action/page_action_icon_view.cc
,
Jun 8 2018
,
Jun 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/23ea4334967e4841d7fad9b4f557e5fecd813c4d commit 23ea4334967e4841d7fad9b4f557e5fecd813c4d Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Jun 08 23:26:54 2018 Omnibox Refresh: Fix focus ring hide behavior when popup opens This CL ensures that the focus ring is shown/hidden properly when the popup is opened/closed by scheduling a paint operation when the popup opens and closes. Bug: 823535 Change-Id: I19303bebc5b64a91f3497d3898e805a81dbacf31 Reviewed-on: https://chromium-review.googlesource.com/1091187 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Bret Sepulveda <bsep@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#565776} [modify] https://crrev.com/23ea4334967e4841d7fad9b4f557e5fecd813c4d/chrome/browser/ui/views/location_bar/location_bar_view.cc
,
Jun 11 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 11 2018
,
Jun 11 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0830cdf01de84c9af5f575add7b511beeba301a2 commit 0830cdf01de84c9af5f575add7b511beeba301a2 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jun 13 15:12:44 2018 Omnibox UI Refresh: Align Omnibox text to new Suggestion text The goal of the CL is to re-align the Omnibox text to match the new Refresh spec. In the new Refresh spec, the text has a different indentation depending on whether or not the popup is open, so this CL resets the insets of the OmniboxViewViews whenever the popup opens or closes. It also updates LocationBarView to no longer ignore the left-inset value of the OmniboxViewViews. We used to also ignore the right-inset value, but we stopped doing that recently [1]. After no longer ignoring the left-inset value, we update the non-Refresh value to the correct value of 0 (to keep things looking the same), and update the Refresh value to 2, which matches mocks. We are updating the inset of the OmniboxViewViews textfield instead of the bounds of the textfield because we want the whole area to remain a clickable I-beam, even if we indent the text a bit. [1] https://codereview.chromium.org/2642893002/diff/100001/chrome/browser/ui/views/location_bar/location_bar_view.cc Bug: 849779 , 823535 Change-Id: I8c2a9e4934a78efec0d3d6cd60c63181be2d92c4 Reviewed-on: https://chromium-review.googlesource.com/1096491 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#566834} [modify] https://crrev.com/0830cdf01de84c9af5f575add7b511beeba301a2/chrome/browser/ui/views/harmony/chrome_layout_provider.cc [modify] https://crrev.com/0830cdf01de84c9af5f575add7b511beeba301a2/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/0830cdf01de84c9af5f575add7b511beeba301a2/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/0830cdf01de84c9af5f575add7b511beeba301a2/chrome/browser/ui/views/omnibox/omnibox_view_views.h
,
Jun 14 2018
,
Jun 15 2018
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5 commit a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Jun 19 16:40:22 2018 Omnibox UI Refresh: Make 16 elevation shadow that matches specs This makes the 16 elevation shadow for the Omnibox match the specs given for MD Refresh. This CL also divorces the Omnibox shadows from the Aura window shadow drawing logic. Bug: 845305 , 823535 Change-Id: Ibbda26b465b528805a1be591a23a20cc09df7643 Reviewed-on: https://chromium-review.googlesource.com/1100006 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#568489} [modify] https://crrev.com/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc [modify] https://crrev.com/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.cc [modify] https://crrev.com/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5/chrome/browser/ui/views/omnibox/rounded_omnibox_results_frame.h [modify] https://crrev.com/a2fa3276b5ef5c0ba7a562e8448de95bfcba51e5/ui/gfx/shadow_value.cc
,
Jun 19 2018
The main thrust of this work is complete now. Mac still needs work, but sdy@ is working on that in 836829.
,
Jun 26 2018
Issue 728844 has been merged into this issue.
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e41d3cc0b23587b7dde5ebad20a69227b341de28 commit e41d3cc0b23587b7dde5ebad20a69227b341de28 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Jun 26 23:56:49 2018 [Merge to M68] Omnibox UI Refresh: Always show the focus ring when focused This merge comprises of the following two CLs together, since the intermediate state is broken: Omnibox UI Refresh: Always show the focus ring when focused Previously, we only showed the focus ring on certain platforms (Mac) or keyboard accessibility mode. Now, following the new Material Refresh guidelines, we display the focus ring on the Omnibox whenever it is focused (and the popup is hidden). This also simplifies the code a bit, as keyboard accessibility mode is no longer relevant. Bug: 844263 , 823535 Change-Id: Ic5133463f6e67af5ef6f0512034938e4a19a9972 Reviewed-on: https://chromium-review.googlesource.com/1069690 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#562979} (cherry picked from commit be652b796af75662f2d0dc64e152d8c90b14f271) Omnibox UI Refresh: Fix legacy focus ring for keyboard accessibility Previously we displayed a legacy focus ring if Material Refresh was off for full keyboard accessibilty mode on all platforms. The CL below accidentally removed it: https://chromium-review.googlesource.com/c/chromium/src/+/1069690 Now we are adding it back with a partial revert - and a refactor to make the logic overall more clear. We can remove everything we are adding in this CL once Material Refresh is permanent and launched. (cherry picked from commit 479e0af586ef7a0b0770312a9f03a13aa72910c2) Bug: 848641 , 844263 , 823535 Change-Id: Ife78cd9660e38f16525b55044ee0a231101534a6 Reviewed-on: https://chromium-review.googlesource.com/1082794 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#563814} Reviewed-on: https://chromium-review.googlesource.com/1115432 Reviewed-by: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/branch-heads/3440@{#543} Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733} [modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/location_bar/location_bar_view.h [modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/chrome/browser/ui/views/toolbar/toolbar_view.cc [modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/ui/views/controls/focus_ring.cc [modify] https://crrev.com/e41d3cc0b23587b7dde5ebad20a69227b341de28/ui/views/controls/focus_ring.h
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe55ead60654aebd0f12bd53f0f6785e1c3741a7 commit fe55ead60654aebd0f12bd53f0f6785e1c3741a7 Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jun 27 18:27:54 2018 Revert "Omnibox UI Refresh: Align Omnibox text to new Suggestion text" This reverts commit 0830cdf01de84c9af5f575add7b511beeba301a2. Reason for revert: UX direction change. Original change's description: > Omnibox UI Refresh: Align Omnibox text to new Suggestion text > > The goal of the CL is to re-align the Omnibox text to match the new > Refresh spec. > > In the new Refresh spec, the text has a different indentation depending > on whether or not the popup is open, so this CL resets the insets > of the OmniboxViewViews whenever the popup opens or closes. > > It also updates LocationBarView to no longer ignore the left-inset > value of the OmniboxViewViews. We used to also ignore the right-inset > value, but we stopped doing that recently [1]. > > After no longer ignoring the left-inset value, we update the > non-Refresh value to the correct value of 0 (to keep things looking > the same), and update the Refresh value to 2, which matches mocks. > > We are updating the inset of the OmniboxViewViews textfield instead of > the bounds of the textfield because we want the whole area to remain a > clickable I-beam, even if we indent the text a bit. > > [1] https://codereview.chromium.org/2642893002/diff/100001/chrome/browser/ui/views/location_bar/location_bar_view.cc > > Bug: 849779 , 823535 > Change-Id: I8c2a9e4934a78efec0d3d6cd60c63181be2d92c4 > Reviewed-on: https://chromium-review.googlesource.com/1096491 > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> > Commit-Queue: Tommy Li <tommycli@chromium.org> > Cr-Commit-Position: refs/heads/master@{#566834} TBR=sky@chromium.org,tommycli@chromium.org,jdonnelly@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 849779 , 823535 Change-Id: Icf9e2f004042be6f25b9a86d23f269f5ebf0fd90 Reviewed-on: https://chromium-review.googlesource.com/1117218 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#570847} [modify] https://crrev.com/fe55ead60654aebd0f12bd53f0f6785e1c3741a7/chrome/browser/ui/views/harmony/chrome_layout_provider.cc [modify] https://crrev.com/fe55ead60654aebd0f12bd53f0f6785e1c3741a7/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/fe55ead60654aebd0f12bd53f0f6785e1c3741a7/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/fe55ead60654aebd0f12bd53f0f6785e1c3741a7/chrome/browser/ui/views/omnibox/omnibox_view_views.h
,
Jun 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b815244a60fd4419607e50a3725ade3b53b1b480 commit b815244a60fd4419607e50a3725ade3b53b1b480 Author: Tommy Li <tommycli@chromium.org> Date: Thu Jun 28 17:04:33 2018 Reland "Omnibox UI Refresh: Align Omnibox text to new Suggestion text" This reverts commit fe55ead60654aebd0f12bd53f0f6785e1c3741a7. Reason for revert: UX direction change. Original change's description: > Revert "Omnibox UI Refresh: Align Omnibox text to new Suggestion text" > > This reverts commit 0830cdf01de84c9af5f575add7b511beeba301a2. > > Reason for revert: UX direction change. > > Original change's description: > > Omnibox UI Refresh: Align Omnibox text to new Suggestion text > > > > The goal of the CL is to re-align the Omnibox text to match the new > > Refresh spec. > > > > In the new Refresh spec, the text has a different indentation depending > > on whether or not the popup is open, so this CL resets the insets > > of the OmniboxViewViews whenever the popup opens or closes. > > > > It also updates LocationBarView to no longer ignore the left-inset > > value of the OmniboxViewViews. We used to also ignore the right-inset > > value, but we stopped doing that recently [1]. > > > > After no longer ignoring the left-inset value, we update the > > non-Refresh value to the correct value of 0 (to keep things looking > > the same), and update the Refresh value to 2, which matches mocks. > > > > We are updating the inset of the OmniboxViewViews textfield instead of > > the bounds of the textfield because we want the whole area to remain a > > clickable I-beam, even if we indent the text a bit. > > > > [1] https://codereview.chromium.org/2642893002/diff/100001/chrome/browser/ui/views/location_bar/location_bar_view.cc > > > > Bug: 849779 , 823535 > > Change-Id: I8c2a9e4934a78efec0d3d6cd60c63181be2d92c4 > > Reviewed-on: https://chromium-review.googlesource.com/1096491 > > Reviewed-by: Scott Violet <sky@chromium.org> > > Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> > > Commit-Queue: Tommy Li <tommycli@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#566834} > > TBR=sky@chromium.org,tommycli@chromium.org,jdonnelly@chromium.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: 849779 , 823535 > Change-Id: Icf9e2f004042be6f25b9a86d23f269f5ebf0fd90 > Reviewed-on: https://chromium-review.googlesource.com/1117218 > Commit-Queue: Tommy Li <tommycli@chromium.org> > Reviewed-by: Tommy Li <tommycli@chromium.org> > Cr-Commit-Position: refs/heads/master@{#570847} TBR=sky@chromium.org,tommycli@chromium.org,jdonnelly@chromium.org Change-Id: Ib5afb6cc4e892594e3cfdd0c775ae6ef1e4f862f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 849779 , 823535 Reviewed-on: https://chromium-review.googlesource.com/1118785 Reviewed-by: Tommy Li <tommycli@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#571168} [modify] https://crrev.com/b815244a60fd4419607e50a3725ade3b53b1b480/chrome/browser/ui/views/harmony/chrome_layout_provider.cc [modify] https://crrev.com/b815244a60fd4419607e50a3725ade3b53b1b480/chrome/browser/ui/views/location_bar/location_bar_view.cc [modify] https://crrev.com/b815244a60fd4419607e50a3725ade3b53b1b480/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/b815244a60fd4419607e50a3725ade3b53b1b480/chrome/browser/ui/views/omnibox/omnibox_view_views.h
,
Jul 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ddedf9e5d46e1bfa61e8f396646d68df50d1c7af commit ddedf9e5d46e1bfa61e8f396646d68df50d1c7af Author: Wez <wez@chromium.org> Date: Tue Jul 03 02:49:37 2018 Revert "Omnibox UI Refresh: Implement popup open and close opacity animations." This reverts commit 5cab33d1317a67d2512b8677a91611b5b5d18d91. Reason for revert: Speculative revert for issue 857486 which coincides with this change affecting the crashing code path. Original change's description: > Omnibox UI Refresh: Implement popup open and close opacity animations. > > This eases in and out the Omnibox popup opacity over 82ms as it opens > and closes. This matches the animation spec provided by UX. > > Bug: 823535 > Change-Id: Ic2add1f57f63ae6d043812f2bd5a766f84d302c9 > Reviewed-on: https://chromium-review.googlesource.com/1081198 > Commit-Queue: Tommy Li <tommycli@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Reviewed-by: Ali Juma <ajuma@chromium.org> > Cr-Commit-Position: refs/heads/master@{#564295} TBR=pkasting@chromium.org,ajuma@chromium.org,tommycli@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 823535 , 857486 Change-Id: Ic251311dda8d91dbfddcb9ae7daea86d2d88bb5f Reviewed-on: https://chromium-review.googlesource.com/1123719 Reviewed-by: Wez <wez@chromium.org> Commit-Queue: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#572090} [modify] https://crrev.com/ddedf9e5d46e1bfa61e8f396646d68df50d1c7af/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/ddedf9e5d46e1bfa61e8f396646d68df50d1c7af/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc [modify] https://crrev.com/ddedf9e5d46e1bfa61e8f396646d68df50d1c7af/ui/compositor/closure_animation_observer.cc [modify] https://crrev.com/ddedf9e5d46e1bfa61e8f396646d68df50d1c7af/ui/compositor/closure_animation_observer.h
,
Jul 6
,
Jul 11
I have a WIP CL to re-land the popup opacity animation. Once that's done, this will be fixed again.
,
Jul 12
,
Jul 16
,
Jul 16
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb8ae909edb91ca2feda46fff70b4f96e5303f58 commit eb8ae909edb91ca2feda46fff70b4f96e5303f58 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Jul 17 17:42:36 2018 Reland "Omnibox UI Refresh: Implement popup open and close opacity animations." This is a reland of 5cab33d1317a67d2512b8677a91611b5b5d18d91 In Patchset 2, it also makes the widget insensitive to mouse events while animating closed. This fixes the crash triggered when the user clicks on the results list while the popup is animating closed. It also makes the popup no longer change its highlighted match in response to mouseover events. We also are insensitive to gesture events while closing, but we do not capture them. Original change's description: > Omnibox UI Refresh: Implement popup open and close opacity animations. > > This eases in and out the Omnibox popup opacity over 82ms as it opens > and closes. This matches the animation spec provided by UX. > > Bug: 823535 > Change-Id: Ic2add1f57f63ae6d043812f2bd5a766f84d302c9 > Reviewed-on: https://chromium-review.googlesource.com/1081198 > Commit-Queue: Tommy Li <tommycli@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Reviewed-by: Ali Juma <ajuma@chromium.org> > Cr-Commit-Position: refs/heads/master@{#564295} TBR=ajuma@chromium.org Bug: 823535 , 857486 Change-Id: Ie497eefd007ae30a9098be30f55f63dc51adecc3 Reviewed-on: https://chromium-review.googlesource.com/1138514 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#575706} [modify] https://crrev.com/eb8ae909edb91ca2feda46fff70b4f96e5303f58/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc [modify] https://crrev.com/eb8ae909edb91ca2feda46fff70b4f96e5303f58/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc [modify] https://crrev.com/eb8ae909edb91ca2feda46fff70b4f96e5303f58/ui/compositor/closure_animation_observer.cc [modify] https://crrev.com/eb8ae909edb91ca2feda46fff70b4f96e5303f58/ui/compositor/closure_animation_observer.h
,
Jul 17
Re-closing since we re-landed the opacity animation. Hope it sticks. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by bugdroid1@chromium.org
, Mar 22 2018