867499 - Selecting "copy" from context menu for selected text dismisses selection

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Input, defect, P2)

Description

[juan becerra [:juanb]]

Tested on 2013-04-30 using latest nightly built from http://hg.mozilla.org/mozilla-central/rev/dd0c611a0a27

While testing story in bug 831952 I noticed that tapping on "Copy" in the context menu for selected text dismissed the text selection. In IE the text selection remains after tapping on "Copy."

Steps:
1. Go to https://www.mozilla.org/about/manifesto.en.htm
2. Select a fragment of one of the paragraphs.
3. Tap to summon the context menu
4. Tap on "Copy"

Expected: Text selection remains, such that you can bring up the context menu again and select the other option or expand the selection.

Actual: Text selection is dismissed.

Comment 1

[juan becerra [:juanb]]

It should be https://www.mozilla.org/about/manifesto.en.html

Comment 2

[Allison Naaktgeboren :ally]

:juanb, I understand this now works for you? Is that the case?

Comment 3

[juan becerra [:juanb]]

This does still doesn't work for me using the steps in comment #0. You have to select using touch in order to see the problem. If you select using the mouse then it works, meaning the selection is kept.

Comment 4

[Jim Mathies [:jimm]]

The behavior is currently by design so we need to change what we do after selection. This is a p3, so it can wait.

Comment 5

[Oleg Zasypkin [:azasypkin]]

Attachment: copy.diff

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8372351 [details] [diff] [review]
copy.diff

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

::: browser/metro/base/tests/mochitest/browser_context_menu_tests.js
@@ +917,5 @@
> +  run: function test() {
> +    info(chromeRoot + "browser_context_menu_tests_02.html");
> +    yield addTab(chromeRoot + "browser_context_menu_tests_02.html");
> +
> +    purgeEventQueue();

I don't think we need these purge calls anymore. I think you can take this out without affecting the test.

Comment 7

[Oleg Zasypkin [:azasypkin]]

Attachment: copy.diff

Thanks for review! Yeah, seems that purgeEventQueue call is redundant.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/3d58bca0922a

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/3d58bca0922a

Comment 10

[Kamil Jozwiak [:kjozwiak]]

Verified the following issue using the latest Nightly, used build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-11-03-02-01-mozilla-central/

Reproduced the original issue from comment #0 using the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-05-03-02-03-mozilla-central/

- Ensured that after taping on "Copy" via the context menu, the text that was selected via the grippers remains selected
- Ensured that after taping on "Copy Link" via the context menu, the text that was selected via the grippers remains selected
- Ensured that everything that was selected, pasted correctly (ensured that text was not missing once paste was used)
- Ensured that you can scroll through the website with the text selected without any issues
- Ensured that when you scroll through the website, the grippers disappeared and reappeared once scrolling has stopped (ensured they're selectable)
- Ensured that you can resize the previous selection via the grippers without any issues
- Ensured that you can zoom using the pinch gesture with the text still highlighted and made sure you can resize the selection while zoomed in
- Ensured that switching between different metro applications still has the text selected and the grippers visible
- Ensured that you can use the grippers after switching between different metro applications
- Went through all of the above test cases using different variations of snapped view