1181418 - [Text Selection] [cut/copy/paste] cut/copy options are always shown on CCP bubble

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Jeremy Chen ([e-mail])]

I just flashed my flame with the latest mozilla-central pvt-build and found that CCP short cut mode is not working.

##### STR
1. Open sms app and press the pan icon to create a new message.
(you can open any app to help you get into a text editor)
2. Input some text, select them, and copy them
3. Tap on any empty space in an editor

##### Expect
CCP get into short cut mode with only paste command available in pop-up bubble

##### Actual
cut/copy commands are shown in pop-up bubble as well

##### PVT Version info.
Build ID               20150707160207
Gaia Revision          86af137c7f4c41c1185e609339002ab47fd6c640
Gaia Date              2015-07-07 17:02:07
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/9340658848d1
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150707.193318
Firmware Date          Tue Jul  7 19:33:30 EDT 2015
Bootloader             L1TC10011880

Comment 1

[Jeremy Chen ([e-mail])]

I flashed w/ mozilla-b2g37_v2_2 pvt-build and the short cut mode works as usual. Looks like v2.2 is not affected.

##### PVT Version info.
Build ID               20150707162503
Gaia Revision          ea11f422b687a982f0a961c9aea7858066561707
Gaia Date              2015-07-02 23:37:50
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/52d19a805d35
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150707.202802
Firmware Date          Tue Jul  7 20:28:14 EDT 2015
Bootloader             L1TC10011880

Comment 2

[Jeremy Chen ([e-mail])]

Not sure if this is related to Bug 1180872.

Comment 3

[Jeremy Chen ([e-mail])]

Attachment: Disable Cut/Copy options on selection bubble when in collapsed mode (v1)

For now, our UI looks a bit weird since we always have cut/copy options in collapsed mode (in shortcut mode too). Main reason could be that we will query isCommandEnabled for cut/copy and these two values are always true. (see Bug 1162952). In Bug 1180872, it seems like we're not going to disable cut option when selecting on Non-Editable text for now. But, to my knowledge, it seems that we're not firing cut/copy/paste event as spec said when clicking cut/copy/paste buttons on bubble. Instead, we send msg to content and let gecko handle cut/copy/paste (see [1]).

I think maybe we can disable cut/copy options in collapsed mode to improve UX for now. In the future, if clipboard API in FireFox can support all three cut/copy/paste events, we can then come up with a plan for a new UI spec or something.

[1] https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsIDocShell.idl#990

Comment 4

[Jeremy Chen ([e-mail])]

Attachment: video of actual result

Comment 5

[Jeremy Chen ([e-mail])]

Attachment: video of expected result

Comment 6

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8635963 [details] [diff] [review]
Disable Cut/Copy options on selection bubble when in collapsed mode (v1)

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

Looks good.

Comment 7

[Jeremy Chen ([e-mail])]

Attachment: Part1: Send selectionEditable info to app_text_selection_dialog (v1)

From gecko, we can aware whether the selected frame is editable or not. Let's send this info to app_text_selection_dialog in Gaia system app, so selection bubble could only show functional command options.

TY, can you take a look at this? Thank you. :)

Comment 8

[GitHub Autolander Bot]

Attachment: Part2: [gaia] chenpighead:1181418 > mozilla-b2g:master

Comment 9

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8641515 [details] [diff] [review]
Part1: Send selectionEditable info to app_text_selection_dialog (v1)

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

::: layout/base/AccessibleCaretManager.cpp
@@ +941,5 @@
>      init.mSelectionVisible = true;
>    }
>  
> +  // Send isEditable info w/ event detail. This info can help determine
> +  // whether to show cut command on selection dialog or not

Please add a period at the end of the sentence.

@@ +946,5 @@
> +  Element* editingHost = nullptr;
> +  if (commonAncestorFrame) {
> +    editingHost = commonAncestorFrame->GetContent()->GetEditingHost();
> +  }
> +  init.mSelectionEditable = editingHost ? true : false;

Nit: init.mSelectionEditable = commonAncestorFrame && commonAncestorFrame->GetContent()->GetEditingHost();

Comment 10

[Jeremy Chen ([e-mail])]

Attachment: Part1: Send selectionEditable info to app_text_selection_dialog (v2)

Address :TYLin's comments.

:kanru, could you review BrowserElement related files?
:smaug, could you review CaretStateChangedEvent.webidl part?
Thank you.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8641626 [details] [diff] [review]
Part1: Send selectionEditable info to app_text_selection_dialog (v2)

r+ (or sr+) for the .webidl

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8641626 [details] [diff] [review]
Part1: Send selectionEditable info to app_text_selection_dialog (v2)

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

r=me for AccessibleCaretManager.

Comment 13

[Jeremy Chen ([e-mail])]

Hi Tim, could you take a look at the Gaia patch? Thank you.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8641516 [details] [review]
Part2: [gaia] chenpighead:1181418 > mozilla-b2g:master

I am very sorry for the delayed review.

Comment 15

[Jeremy Chen ([e-mail])]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e1aa6a5cedf
Please check-in Part1 for Gecko and Part2 for Gaia, thanks.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2848ae508c5d

Comment 17

[Jeremy Chen ([e-mail])]

Please leave this bug open until the gaia part patch is landed.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2848ae508c5d

Comment 19

[Jeremy Chen ([e-mail])]

Try looks positive. Please check-in Part2 for Gaia, thanks.

Comment 20

[Carsten Book [:Tomcat]]

landed on gaia https://github.com/mozilla-b2g/gaia/commit/474784fc494519a0fab866407c0215aacf984ed9