Consider entity length when deciding whether to show actions
Currently, we show the highest-confidence action, regardless of how much space it takes up in the total copied text. This leads to some strange behaviors like showing a calendar action if a date is shown anywhere in the text. This change adds a minimum proportion (currently 80%); if the text link takes up less than that proportion of the total copied text, it is ignored. Bug: 270445121 Bug: 270421442 Fix: 270445121 Test: manual (using an article headline+date, as in the bug); atest ClipboardOverlayUtilsTest Change-Id: I99b9c48aae121244e01f8783d9eb8068d3c12fe3
This commit is contained in:
@@ -363,15 +363,15 @@ public class ClipboardOverlayController implements ClipboardListener.ClipboardOv
|
||||
|
||||
private void classifyText(ClipboardModel model) {
|
||||
mBgExecutor.execute(() -> {
|
||||
Optional<RemoteAction> remoteAction = mClipboardUtils.getAction(
|
||||
model.getText(), model.getTextLinks(), model.getSource());
|
||||
Optional<RemoteAction> remoteAction =
|
||||
mClipboardUtils.getAction(model.getTextLinks(), model.getSource());
|
||||
if (model.equals(mClipboardModel)) {
|
||||
remoteAction.ifPresent(action -> {
|
||||
mClipboardLogger.logUnguarded(CLIPBOARD_OVERLAY_ACTION_SHOWN);
|
||||
mView.setActionChip(action, () -> {
|
||||
mView.post(() -> mView.setActionChip(action, () -> {
|
||||
mClipboardLogger.logSessionComplete(CLIPBOARD_OVERLAY_ACTION_TAPPED);
|
||||
animateOut();
|
||||
});
|
||||
}));
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -39,6 +39,9 @@ import javax.inject.Inject;
|
||||
|
||||
class ClipboardOverlayUtils {
|
||||
|
||||
// minimum proportion of entire text an entity must take up, to be considered for smart actions
|
||||
private static final float MINIMUM_ENTITY_PROPORTION = .8f;
|
||||
|
||||
private final TextClassifier mTextClassifier;
|
||||
|
||||
@Inject
|
||||
@@ -65,19 +68,23 @@ class ClipboardOverlayUtils {
|
||||
return false;
|
||||
}
|
||||
|
||||
public Optional<RemoteAction> getAction(CharSequence text, TextLinks textLinks, String source) {
|
||||
return getActions(text, textLinks).stream().filter(remoteAction -> {
|
||||
public Optional<RemoteAction> getAction(TextLinks textLinks, String source) {
|
||||
return getActions(textLinks).stream().filter(remoteAction -> {
|
||||
ComponentName component = remoteAction.getActionIntent().getIntent().getComponent();
|
||||
return component != null && !TextUtils.equals(source, component.getPackageName());
|
||||
}).findFirst();
|
||||
}
|
||||
|
||||
private ArrayList<RemoteAction> getActions(CharSequence text, TextLinks textLinks) {
|
||||
private ArrayList<RemoteAction> getActions(TextLinks textLinks) {
|
||||
ArrayList<RemoteAction> actions = new ArrayList<>();
|
||||
for (TextLinks.TextLink link : textLinks.getLinks()) {
|
||||
TextClassification classification = mTextClassifier.classifyText(
|
||||
text, link.getStart(), link.getEnd(), null);
|
||||
actions.addAll(classification.getActions());
|
||||
// skip classification for incidental entities
|
||||
if (link.getEnd() - link.getStart()
|
||||
>= textLinks.getText().length() * MINIMUM_ENTITY_PROPORTION) {
|
||||
TextClassification classification = mTextClassifier.classifyText(
|
||||
textLinks.getText(), link.getStart(), link.getEnd(), null);
|
||||
actions.addAll(classification.getActions());
|
||||
}
|
||||
}
|
||||
return actions;
|
||||
}
|
||||
@@ -92,9 +99,13 @@ class ClipboardOverlayUtils {
|
||||
private ArrayList<RemoteAction> getActions(ClipData.Item item) {
|
||||
ArrayList<RemoteAction> actions = new ArrayList<>();
|
||||
for (TextLinks.TextLink link : item.getTextLinks().getLinks()) {
|
||||
TextClassification classification = mTextClassifier.classifyText(
|
||||
item.getText(), link.getStart(), link.getEnd(), null);
|
||||
actions.addAll(classification.getActions());
|
||||
// skip classification for incidental entities
|
||||
if (link.getEnd() - link.getStart()
|
||||
>= item.getText().length() * MINIMUM_ENTITY_PROPORTION) {
|
||||
TextClassification classification = mTextClassifier.classifyText(
|
||||
item.getText(), link.getStart(), link.getEnd(), null);
|
||||
actions.addAll(classification.getActions());
|
||||
}
|
||||
}
|
||||
return actions;
|
||||
}
|
||||
|
||||
@@ -446,7 +446,7 @@ public class ClipboardOverlayControllerTest extends SysuiTestCase {
|
||||
mFeatureFlags.set(CLIPBOARD_REMOTE_BEHAVIOR, true);
|
||||
when(mClipboardUtils.isRemoteCopy(any(Context.class), any(ClipData.class), anyString()))
|
||||
.thenReturn(true);
|
||||
when(mClipboardUtils.getAction(any(CharSequence.class), any(TextLinks.class), anyString()))
|
||||
when(mClipboardUtils.getAction(any(TextLinks.class), anyString()))
|
||||
.thenReturn(Optional.of(Mockito.mock(RemoteAction.class)));
|
||||
when(mClipboardOverlayView.post(any(Runnable.class))).thenAnswer(new Answer<Object>() {
|
||||
@Override
|
||||
|
||||
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.anyInt;
|
||||
import static org.mockito.ArgumentMatchers.anyString;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.ArgumentMatchers.isNull;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@@ -77,6 +78,74 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
|
||||
|
||||
@Test
|
||||
public void test_getAction_noLinks_returnsEmptyOptional() {
|
||||
Optional<RemoteAction> action =
|
||||
mClipboardUtils.getAction(Mockito.mock(TextLinks.class), "abc");
|
||||
|
||||
assertTrue(action.isEmpty());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_returnsFirstLink() {
|
||||
TextLinks links = getFakeTextLinksBuilder().build();
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
TextClassification classificationA = Mockito.mock(TextClassification.class);
|
||||
when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
|
||||
TextClassification classificationB = Mockito.mock(TextClassification.class);
|
||||
when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
|
||||
when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
|
||||
classificationA, classificationB);
|
||||
|
||||
RemoteAction result = mClipboardUtils.getAction(links, "test").orElse(null);
|
||||
|
||||
assertEquals(actionA, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_skipsMatchingComponent() {
|
||||
TextLinks links = getFakeTextLinksBuilder().build();
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
TextClassification classificationA = Mockito.mock(TextClassification.class);
|
||||
when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
|
||||
TextClassification classificationB = Mockito.mock(TextClassification.class);
|
||||
when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
|
||||
when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
|
||||
classificationA, classificationB);
|
||||
|
||||
RemoteAction result = mClipboardUtils.getAction(links, "abc").orElse(null);
|
||||
|
||||
assertEquals(actionB, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_skipsShortEntity() {
|
||||
TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
|
||||
final Map<String, Float> scores = new ArrayMap<>();
|
||||
scores.put(TextClassifier.TYPE_EMAIL, 1f);
|
||||
textLinks.addLink(20, 22, scores);
|
||||
textLinks.addLink(0, 22, scores);
|
||||
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
TextClassification classificationA = Mockito.mock(TextClassification.class);
|
||||
when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
|
||||
TextClassification classificationB = Mockito.mock(TextClassification.class);
|
||||
when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
|
||||
when(mTextClassifier.classifyText(anyString(), eq(20), eq(22), isNull())).thenReturn(
|
||||
classificationA);
|
||||
when(mTextClassifier.classifyText(anyString(), eq(0), eq(22), isNull())).thenReturn(
|
||||
classificationB);
|
||||
|
||||
RemoteAction result = mClipboardUtils.getAction(textLinks.build(), "test").orElse(null);
|
||||
|
||||
assertEquals(actionB, result);
|
||||
}
|
||||
|
||||
// TODO(b/267162944): Next four tests (marked "legacy") are obsolete once
|
||||
// CLIPBOARD_MINIMIZED_LAYOUT flag is released and removed
|
||||
@Test
|
||||
public void test_getAction_noLinks_returnsEmptyOptional_legacy() {
|
||||
ClipData.Item item = new ClipData.Item("no text links");
|
||||
item.setTextLinks(Mockito.mock(TextLinks.class));
|
||||
|
||||
@@ -86,8 +155,8 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_returnsFirstLink() {
|
||||
when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinks());
|
||||
public void test_getAction_returnsFirstLink_legacy() {
|
||||
when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinksBuilder().build());
|
||||
when(mClipDataItem.getText()).thenReturn("");
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
@@ -98,14 +167,14 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
|
||||
when(mTextClassifier.classifyText(anyString(), anyInt(), anyInt(), isNull())).thenReturn(
|
||||
classificationA, classificationB);
|
||||
|
||||
RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "def").orElse(null);
|
||||
RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "test").orElse(null);
|
||||
|
||||
assertEquals(actionA, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_skipsMatchingComponent() {
|
||||
when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinks());
|
||||
public void test_getAction_skipsMatchingComponent_legacy() {
|
||||
when(mClipDataItem.getTextLinks()).thenReturn(getFakeTextLinksBuilder().build());
|
||||
when(mClipDataItem.getText()).thenReturn("");
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
@@ -121,6 +190,33 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
|
||||
assertEquals(actionB, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_getAction_skipsShortEntity_legacy() {
|
||||
TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
|
||||
final Map<String, Float> scores = new ArrayMap<>();
|
||||
scores.put(TextClassifier.TYPE_EMAIL, 1f);
|
||||
textLinks.addLink(20, 22, scores);
|
||||
textLinks.addLink(0, 22, scores);
|
||||
|
||||
when(mClipDataItem.getTextLinks()).thenReturn(textLinks.build());
|
||||
when(mClipDataItem.getText()).thenReturn(textLinks.build().getText());
|
||||
|
||||
RemoteAction actionA = constructRemoteAction("abc");
|
||||
RemoteAction actionB = constructRemoteAction("def");
|
||||
TextClassification classificationA = Mockito.mock(TextClassification.class);
|
||||
when(classificationA.getActions()).thenReturn(Lists.newArrayList(actionA));
|
||||
TextClassification classificationB = Mockito.mock(TextClassification.class);
|
||||
when(classificationB.getActions()).thenReturn(Lists.newArrayList(actionB));
|
||||
when(mTextClassifier.classifyText(anyString(), eq(20), eq(22), isNull())).thenReturn(
|
||||
classificationA);
|
||||
when(mTextClassifier.classifyText(anyString(), eq(0), eq(22), isNull())).thenReturn(
|
||||
classificationB);
|
||||
|
||||
RemoteAction result = mClipboardUtils.getAction(mClipDataItem, "test").orElse(null);
|
||||
|
||||
assertEquals(actionB, result);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test_extra_withPackage_returnsTrue() {
|
||||
PersistableBundle b = new PersistableBundle();
|
||||
@@ -184,12 +280,12 @@ public class ClipboardOverlayUtilsTest extends SysuiTestCase {
|
||||
return action;
|
||||
}
|
||||
|
||||
private static TextLinks getFakeTextLinks() {
|
||||
TextLinks.Builder textLinks = new TextLinks.Builder("test");
|
||||
private static TextLinks.Builder getFakeTextLinksBuilder() {
|
||||
TextLinks.Builder textLinks = new TextLinks.Builder("test text of length 22");
|
||||
final Map<String, Float> scores = new ArrayMap<>();
|
||||
scores.put(TextClassifier.TYPE_EMAIL, 1f);
|
||||
textLinks.addLink(0, 0, scores);
|
||||
textLinks.addLink(0, 0, scores);
|
||||
return textLinks.build();
|
||||
textLinks.addLink(0, 22, scores);
|
||||
textLinks.addLink(0, 22, scores);
|
||||
return textLinks;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user