858359 - Work - Remove Selectable context menu options on input and textareas when there is text and no selection

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Browser, defect)

Description

[Brian R. Bondy [:bbondy]]

The story in bug 844658 does not list select and select all. I'm not sure what select does but it seems like it does nothing. Select all has problems with the selection markers when using touch.

These should both be removed.

Comment 1

[Brian R. Bondy [:bbondy]]

So the purpose of select is to add a selection marker if you long tap in a box with text.

Comment 2

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

I think I have to update a test or two, I'll be attaching another patch with that.

Comment 3

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1 - tests

Unfortunately even before my patches the tests for context menus fail with a timeout and some other failures, I think related to needing a yield (wait for text on clipboard) when copying text on the clipboard.   

I can get other browser chrome mochitests working, and the tests do run in the immersive environment so I'm running them correctly.

I can look into those failures later but I think this is the changes needed for this bug. If they don't fail for you maybe you could take this patch for a spin?

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 734569 [details] [diff] [review]
Patch v1 - tests

Review of attachment 734569 [details] [diff] [review]:
-----------------------------------------------------------------

I get the copy text failures too, I think the copy commands were changed to async a while back.

::: browser/metro/base/tests/mochitest/browser_context_menu_tests.js
@@ +184,5 @@
>      yield popupPromise;
>      ok(popupPromise && !(popupPromise instanceof Error), "promise error");
>  
>      let string = SpecialPowers.getClipboardData("text/unicode");
> +    ok(string === "hello", "copied selected text (hell0 === " + string + ")");

hell0 typo?

@@ +314,5 @@
>      purgeEventQueue();
>    }
>  });
>  
> +/*

why are all the image tests commented out?

@@ +398,5 @@
>      menuItem = document.getElementById("context-copy-image");
>      ok(menuItem, "menu item exists");
>      ok(!menuItem.hidden, "menu item visible");
>      popupPromise = waitForEvent(document, "popuphidden");
> +    EventUtils.synthesizeMouseAtCenter(menuItem, 10, 10, {}, win);

nix the coordinates here?

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1' - tests

Hey Jim sorry I attached a patch I was investigating test failures with in my m-c dir instead of the one I wanted to apply in a different dir.

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 734382 [details] [diff] [review]
Patch v1.

no problem!

Comment 7

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3e849b9ea6f
https://hg.mozilla.org/integration/mozilla-inbound/rev/cee523a84f6b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cee523a84f6b
https://hg.mozilla.org/mozilla-central/rev/f3e849b9ea6f