Skip to content

Commit

Permalink
[android][dropdown 1/n] - Refactor Text Selection Menu Population
Browse files Browse the repository at this point in the history
The previous text selection menu logic for populating items was spread out, hard to understand, and heavily reliant on Android menu APIs. It was also used in two different places SelectionPopupControllerImpl & FloatingPastePopupMenu. Given that we are now planning to add a third menu type (see attached bug), it would be good to somehow centralize this logic while also making it generic enough for use with any menu rendering API. This means doing a few things.

1. Decoupling the menu item provision from the Android menu APIs.
2. Refactoring additional menu item providers (i.e. autofill) to be more generic and providing them to the SelectionPopupController.
3. Refactoring SelectionPopupControllerImpl && FloatingPastePopupMenu to use these new APIs.

All of this will need to be done while ensuring the current behavior does not change. This means when and in what order items show up should remain the same.

Follow up CLs starting at https://chromium-review.googlesource.com/c/chromium/src/+/4507323 will be used to implement a drop-down style menu that will leverage this refactor.

Bug: 1428022
Change-Id: Ic7ad00eb67275b9e4ecb8c8b1324bffb48e61b42
Low-Coverage-Reason: No logic changes in the classes mentioned, only a refactor. Also SelectionPopupControllerImpl has existing test coverage in SelectionPopupControllerTest (unit) & ContentTextSelectionTest (integration) which have been ran and pass locally.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4547564
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Commit-Queue: Wayne Jackson Jr. <wbjacksonjr@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1152130}
  • Loading branch information
Wayne Jackson Jr authored and Chromium LUCI CQ committed Jun 1, 2023
1 parent cbba035 commit 2a2f22a
Show file tree
Hide file tree
Showing 26 changed files with 1,049 additions and 587 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public boolean onActionItemClicked(ActionMode mode, MenuItem item) {

int groupId = item.getGroupId();

if (groupId == R.id.select_action_menu_text_processing_menus) {
if (groupId == R.id.select_action_menu_text_processing_items) {
processText(item.getIntent());
// The ActionMode is not dismissed to match the behavior with
// TextView in Android M.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@
import org.chromium.base.task.AsyncTask;
import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits;
import org.chromium.components.autofill.AutofillActionModeCallback;
import org.chromium.components.autofill.AutofillProvider;
import org.chromium.components.autofill.AutofillSelectionMenuItemProvider;
import org.chromium.components.content_capture.OnscreenContentProvider;
import org.chromium.components.embedder_support.util.WebResourceResponseInfo;
import org.chromium.components.navigation_interception.InterceptNavigationDelegate;
Expand Down Expand Up @@ -1172,8 +1172,8 @@ private void initializeAutofillProviderIfNecessary() {
mAutofillProvider.setWebContents(mWebContents);
}
SelectionPopupController.fromWebContents(mWebContents)
.setNonSelectionActionModeCallback(
new AutofillActionModeCallback(mContext, mAutofillProvider));
.setNonSelectionAdditionalMenuItemProvider(
new AutofillSelectionMenuItemProvider(mContext, mAutofillProvider));
AwContentsJni.get().initializeAndroidAutofill(mNativeAwContents);
}

Expand Down
22 changes: 0 additions & 22 deletions chrome/android/expectations/lint-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,28 +89,6 @@
column="27"/>
</issue>

<issue
id="NewApi"
message="Call requires API level 26 (current min is 24): `shouldQueryAutofillSuggestion`"
errorLine1=" if (mAutofillMenuItemTitle != 0 &amp;&amp; mAutofillProvider.shouldQueryAutofillSuggestion()) {"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../components/android_autofill/browser/java/src/org/chromium/components/autofill/AutofillActionModeCallback.java"
line="43"
column="62"/>
</issue>

<issue
id="NewApi"
message="Call requires API level 26 (current min is 24): `queryAutofillSuggestion`"
errorLine1=" mAutofillProvider.queryAutofillSuggestion();"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="../../components/android_autofill/browser/java/src/org/chromium/components/autofill/AutofillActionModeCallback.java"
line="55"
column="31"/>
</issue>

<issue
id="NewApi"
message="Call requires API level 26 (current min is 24): `deleteLegacyChannels`"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public boolean onPrepareActionMode(ActionMode mode, Menu menu) {
Set<String> launchers = getPackageNames(PackageManagerUtils.queryAllLaunchersInfo());
for (int i = 0; i < menu.size(); i++) {
MenuItem item = menu.getItem(i);
if (item.getGroupId() != R.id.select_action_menu_text_processing_menus
if (item.getGroupId() != R.id.select_action_menu_text_processing_items
|| item.getIntent() == null || item.getIntent().getComponent() == null) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private ResolveInfo createResolveInfo(String packageName) {
}

private void addMenuItem(Menu menu, int order, String packageName) {
menu.add(R.id.select_action_menu_text_processing_menus, Menu.NONE, order, "title")
menu.add(R.id.select_action_menu_text_processing_items, Menu.NONE, order, "title")
.setIntent(new Intent()
.setAction(Intent.ACTION_PROCESS_TEXT)
.setType("text/plain")
Expand Down
2 changes: 1 addition & 1 deletion components/android_autofill/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ android_library("java") {
]
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
sources = [
"java/src/org/chromium/components/autofill/AutofillActionModeCallback.java",
"java/src/org/chromium/components/autofill/AutofillHintsService.java",
"java/src/org/chromium/components/autofill/AutofillManagerWrapper.java",
"java/src/org/chromium/components/autofill/AutofillProvider.java",
"java/src/org/chromium/components/autofill/AutofillProviderUMA.java",
"java/src/org/chromium/components/autofill/AutofillRequest.java",
"java/src/org/chromium/components/autofill/AutofillSelectionMenuItemProvider.java",
"java/src/org/chromium/components/autofill/FormData.java",
"java/src/org/chromium/components/autofill/FormFieldData.java",
"java/src/org/chromium/components/autofill_public/ViewType.java",
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.components.autofill;

import android.content.Context;
import android.os.Build;
import android.view.Menu;
import android.view.MenuItem;

import org.chromium.content_public.browser.AdditionalSelectionMenuItemProvider;
import org.chromium.content_public.browser.SelectionMenuItem;

import java.util.ArrayList;
import java.util.List;

/**
* The class to provide autofill selection context menu items. To match the Android native view
* behavior, the autofill context menu only appears when there is no text selected.
*/
public class AutofillSelectionMenuItemProvider implements AdditionalSelectionMenuItemProvider {
private final AutofillProvider mAutofillProvider;
private final int mAutofillMenuItemTitle;

// using getIdentifier to work around not-exposed framework resource ID
@SuppressWarnings("DiscouragedApi")
public AutofillSelectionMenuItemProvider(Context context, AutofillProvider autofillProvider) {
mAutofillProvider = autofillProvider;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1) {
mAutofillMenuItemTitle = android.R.string.autofill;
} else {
// The string resource was not made public until O MR1, so on O we look it up by name.
mAutofillMenuItemTitle =
context.getResources().getIdentifier("autofill", "string", "android");
}
}

@Override
public List<SelectionMenuItem> getItems() {
List<SelectionMenuItem> autofillItems = new ArrayList<>();
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
if (mAutofillMenuItemTitle != 0 && mAutofillProvider.shouldQueryAutofillSuggestion()) {
SelectionMenuItem autofillItem =
new SelectionMenuItem.Builder(mAutofillMenuItemTitle)
.setId(android.R.id.autofill)
.setOrderInCategory(Menu.CATEGORY_SECONDARY)
.setShowAsActionFlags(MenuItem.SHOW_AS_ACTION_NEVER
| MenuItem.SHOW_AS_ACTION_WITH_TEXT)
.setClickListener(v -> mAutofillProvider.queryAutofillSuggestion())
.build();
autofillItems.add(autofillItem);
}
}
return autofillItems;
}

@Override
public void onMenuDestroyed() {
// no-op
}
}
9 changes: 6 additions & 3 deletions content/public/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ android_resources("content_java_resources") {
"java/res/layout/text_edit_suggestion_item.xml",
"java/res/layout/text_edit_suggestion_list_footer.xml",
"java/res/layout/two_field_date_picker.xml",
"java/res/menu/select_action_menu.xml",
"java/res/values/attrs.xml",
"java/res/values/dimens.xml",
"java/res/values/ids.xml",
"java/res/values/strings.xml",
"java/res/values/styles.xml",
]
Expand Down Expand Up @@ -170,6 +170,7 @@ android_library("content_full_java") {
"//third_party/android_deps:guava_android_java",
"//third_party/androidx:androidx_annotation_annotation_experimental_java",
"//third_party/androidx:androidx_annotation_annotation_java",
"//third_party/androidx:androidx_appcompat_appcompat_resources_java",
"//third_party/androidx:androidx_collection_collection_java",
"//third_party/androidx:androidx_core_core_java",
"//third_party/androidx:androidx_privacysandbox_ads_ads_adservices_java",
Expand Down Expand Up @@ -307,15 +308,14 @@ android_library("content_full_java") {
"java/src/org/chromium/content/browser/remoteobjects/RemoteObjectImpl.java",
"java/src/org/chromium/content/browser/remoteobjects/RemoteObjectInjector.java",
"java/src/org/chromium/content/browser/remoteobjects/RemoteObjectRegistry.java",
"java/src/org/chromium/content/browser/selection/AdditionalMenuItemProvider.java",
"java/src/org/chromium/content/browser/selection/AdditionalMenuItemProviderImpl.java",
"java/src/org/chromium/content/browser/selection/FloatingPastePopupMenu.java",
"java/src/org/chromium/content/browser/selection/LGEmailActionModeWorkaroundImpl.java",
"java/src/org/chromium/content/browser/selection/MagnifierAnimator.java",
"java/src/org/chromium/content/browser/selection/MagnifierSurfaceControl.java",
"java/src/org/chromium/content/browser/selection/MagnifierWrapper.java",
"java/src/org/chromium/content/browser/selection/MagnifierWrapperImpl.java",
"java/src/org/chromium/content/browser/selection/PastePopupMenu.java",
"java/src/org/chromium/content/browser/selection/SelectActionMenuHelper.java",
"java/src/org/chromium/content/browser/selection/SelectionIndicesConverter.java",
"java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java",
"java/src/org/chromium/content/browser/selection/SmartSelectionClient.java",
Expand All @@ -330,6 +330,7 @@ android_library("content_full_java") {
"java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java",
"java/src/org/chromium/content/browser/webid/MDocProviderAndroid.java",
"java/src/org/chromium/content_public/browser/ActionModeCallbackHelper.java",
"java/src/org/chromium/content_public/browser/AdditionalSelectionMenuItemProvider.java",
"java/src/org/chromium/content_public/browser/BrowserContextHandle.java",
"java/src/org/chromium/content_public/browser/ChildProcessLauncherHelper.java",
"java/src/org/chromium/content_public/browser/ClientDataJson.java",
Expand Down Expand Up @@ -372,6 +373,8 @@ android_library("content_full_java") {
"java/src/org/chromium/content_public/browser/SelectAroundCaretResult.java",
"java/src/org/chromium/content_public/browser/SelectionClient.java",
"java/src/org/chromium/content_public/browser/SelectionEventProcessor.java",
"java/src/org/chromium/content_public/browser/SelectionMenuGroup.java",
"java/src/org/chromium/content_public/browser/SelectionMenuItem.java",
"java/src/org/chromium/content_public/browser/SelectionPopupController.java",
"java/src/org/chromium/content_public/browser/SmartClipProvider.java",
"java/src/org/chromium/content_public/browser/SpeechRecognition.java",
Expand Down
79 changes: 0 additions & 79 deletions content/public/android/java/res/menu/select_action_menu.xml

This file was deleted.

16 changes: 16 additions & 0 deletions content/public/android/java/res/values/ids.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<!-- Selection Menu Group IDs -->
<item type="id" name="select_action_menu_assist_items" />
<item type="id" name="select_action_menu_default_items" />
<item type="id" name="select_action_menu_text_processing_items" />

<!-- Selection Menu Items -->
<item type="id" name="select_action_menu_cut" />
<item type="id" name="select_action_menu_copy" />
<item type="id" name="select_action_menu_paste" />
<item type="id" name="select_action_menu_share" />
<item type="id" name="select_action_menu_select_all" />
<item type="id" name="select_action_menu_paste_as_plain_text" />
<item type="id" name="select_action_menu_web_search" />
</resources>
6 changes: 0 additions & 6 deletions content/public/android/java/res/values/styles.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ found in the LICENSE file.
<item name="select_dialog_singlechoice">@android:layout/select_dialog_singlechoice</item>
<item name="select_dialog_multichoice">@android:layout/select_dialog_multichoice</item>
</style>
<style name="SelectActionMenuShare">
<item name="android:icon">?android:attr/actionModeShareDrawable</item>
</style>
<style name="SelectActionMenuWebSearch">
<item name="android:icon">?android:attr/actionModeWebSearchDrawable</item>
</style>
<style name="TextAppearance.SuggestionPrefixOrSuffix">
<item name="android:textColor">?android:attr/textColorSecondary</item>
</style>
Expand Down

0 comments on commit 2a2f22a

Please sign in to comment.