938045 - [Window Management] Implement TextSelectionDiag for Copy&Paste on Firefox OS

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

In order to perform copy and paste after selection, we'll need to display a floating bubble widget that contains copy&paste buttons.

Comment 1

[André Fiedler]

First idea:

Would be nice to have some script inside Gaia to enable the floating bubble widget for certain HTML Elements. I'm thinking about a script like the navigator.mozL10n [1]

Something like:

navigator.mozEnableCopyAndPaste(document.querySelector('p'));

This will show a floating bubble widget on all p-Tags.

----

Or Second idea:

The floating bubble widget should be disable-able via css.  Will this work? ->

header * {
   user-select: none;
}

So we can disable the copy-and-paste functionally on UI elements like tabs aso.


[1] https://github.com/mozilla-b2g/gaia/blob/master/shared/js/l10n.js

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

I'd rather go with the CSS selectors than implement a new non-standard API.

Comment 3

[André Fiedler]

Ok, would be my prefered one too. If this bug has landed, the Gaia Building Blocks [1] should be updated to include the CSS fix.

[1] http://buildingfirefoxos.com/building-blocks/action-menu.html

Comment 4

[Stephany Wilkes]

Keyboard team, please chime in on how this fits with the existing, in-progress work on cursor position (1.3) and text selection and cut/copy/paste for 1.4. How do these things fit together?

Comment 5

[GoodMike]

UX team is working on spec for edit mode after selecting texts. We had floating bubble or change header with edit mode options at the moment. But we are searching for a better approach and welcome any good idea.

Comment 6

[Jan Jongboom [:janjongboom] (inactive)]

I'd go with a floating bubble, the Android header that we have in FF for Android is so incredibly unclear.

Comment 7

[André Fiedler]

+1 for a floating bubble

Comment 8

[Bruce Huang [:bhuang] (inactive)]

(In reply to Stephany Wilkes from comment #4)
> Keyboard team, please chime in on how this fits with the existing,
> in-progress work on cursor position (1.3) and text selection and
> cut/copy/paste for 1.4. How do these things fit together?

This should be part of the copy/paste work, which comes after text selection.  Looking at 1.5 for now, but it's great that this prep work discussion is starting early.

Comment 9

[[:fabrice] Fabrice Desré]

We already have text selection working in input fields, so it's very useful to start with that in 1.4

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Taken for new proposed way to implement this under new window management system.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

Morris I need my device with your WIP gecko patch to work on, thanks.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

We need to find someone to rebase the gaia patch in bug 987040 and add unit tests and put the assets in bug 921965. The layout part will be another bug which is expected to be fixed in stabilization phase but not this week.

Comment 13

[(inactive after 6/18) Alive Kuo [:alive]]

^

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #12)
> We need to find someone to rebase the gaia patch in bug 987040 and add unit
> tests and put the assets in bug 921965. The layout part will be another bug
> which is expected to be fixed in stabilization phase but not this week.

..And I have a device with Morris' gecko patch.

Comment 15

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

George, would you be able to work with Alive on this?

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Layout/assets is here
https://bugzilla.mozilla.org/show_bug.cgi?id=921965

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

Sure, I'll do what comment 12 said.
Take it first.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15)
> George, would you be able to work with Alive on this?

Comment 18

[howie [:howie]]

blocked by gecko part, move to 2.1

Comment 19

[howie [:howie]]

decision made to work concurrently with gecko part browser api. UI and behavior part is not blocked.

Comment 20

[(Inactive after June) George Duan [:gduan] [:喬智]]

WIP:
https://github.com/cctuan/gaia/commit/c04155618b2e593e705d592b2f5d4bb8f6d55185
still working on: 
1. style modification
2. test case for dialog position

Comment 21

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Hi Alive,
could you kindly help me to review this patch?
Thanks.

Comment 22

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8434831 [details] [review]
PR to master

I just found out there're two API issues that cannot meet UX spec.
1. textualmenu event happens everywhere:
   I cannot distinguish whether user click on menu or other text input by current API, so I can't show effect when user put finger on menu and change background color.
2. When click selectAll button, gecko will emit an textualmenu and only have canSelectAll as true, which and then emit another textualmenu with all four items enabled. So, I cannot position dialog in the center of screen immediately when user click selectAll.

I will put some workarounds on gaia.

Comment 23

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Morris,
could you take a look at comment 22?
Thanks.

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

For item2, the root cause is the mousedown event emit to below content very soon then to trigger a dialog with only canPaste option.
(In reply to George Duan [:gduan] [:喬智] from comment #23)
> Hi Morris,
> could you take a look at comment 22?
> Thanks.

Comment 25

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8434831 [details] [review]
PR to master

Hi Alive,
could you help me to review it again?
I've put my workaround to prevent paste event be triggered again. But I have no good solution to item2 of comment 22 so far. I will open a bug for it and discuss with Morris.

Comment 26

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8434831 [details] [review]
PR to master

r+ with nits. Great work! Note the travis is broken in bootstrap and I just rerun it. Be careful to merge.

Comment 27

[(Inactive after June) George Duan [:gduan] [:喬智]]

Offline demo and discussed with Omega from UX.
1. When click selectAll button, sometime a single paste dialog will show before displaying all 4 buttons.
   The reason is textualmenu event with canPaste=true is sent followed by selectall().
   >> We need gecko side to check it.

2. With moving single caret, the dialog cannot show again by clicking the handler.
   >> Known issue from Gecko, need to file a bug for it.

3. The dialog should display while scroll end.
   >> This feature will not be implemented in this state due to gecko limit. We can file a bug if necessary.

4. Carets will interfere url input of browser when the path is too long.
   >> Need a bug to check this issue.

Comment 28

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8434831 [details] [review]
PR to master

Hi Alive,
could you take a look of my patch again?
I've addressed your opinion and change the dialog position mechanism when selectAll based on UX spec.
Thanks.

Comment 29

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8434831 [details] [review]
PR to master

Hi Omega,
as offline discussed and reviewed from you, I've addressed issues as comment 27.
Please let me know if it's suitable or not. Thanks.

Comment 30

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8434831 [details] [review]
PR to master

some nits.

Comment 31

[howie [:howie]]

Hi George, the patch is r+, and as Comment 27 which is already discussed with UX, please commit the code and resolved fix this bug. Thank you!

Comment 32

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks Alive,
merge into master.
https://github.com/mozilla-b2g/gaia/commit/7f7dda12b10ae62b674fa2ebdaeb53dea6f2976c

Comment 33

[(Inactive after June) George Duan [:gduan] [:喬智]]

(In reply to George Duan [:gduan] [:喬智] from comment #27)
> Offline demo and discussed with Omega from UX.
> 1. When click selectAll button, sometime a single paste dialog will show
> before displaying all 4 buttons.
>    The reason is textualmenu event with canPaste=true is sent followed by
> selectall().
>    >> We need gecko side to check it.
> 
bug 1023037
> 2. With moving single caret, the dialog cannot show again by clicking the
> handler.
>    >> Known issue from Gecko, need to file a bug for it.
> 
bug 1023041
> 3. The dialog should display while scroll end.
>    >> This feature will not be implemented in this state due to gecko limit.
> We can file a bug if necessary.
> 
> 4. Carets will interfere url input of browser when the path is too long.
>    >> Need a bug to check this issue.

Comment 34

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

It looks like unit tests failed on TBPL

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=43778d875824
https://tbpl.mozilla.org/php/getParsedLog.php?id=41409812&tree=B2g-Inbound&full=1

George, could you verify and see if we need to back out the patch?

Comment 35

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks,
I'll modify my test cases.

back it out now
be1ac200b791c79cb5d072777732c71c2f5593b5

Comment 36

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master 2

This patch has modified layoutManager of unit test to fixed number, so that the test case related to screen position wound not be affected by env.

waiting for result from tbpl gaia-try.

Comment 37

[Omega Feng [:Omega] [:馮於懋]]

Comment on attachment 8434831 [details] [review]
PR to master

Hi George, here are some feedback:

For current phase:
1. Even a text input has no focus, it should still allow long-press-to-select behavior.
2. When a text input has focus, it can be either long-press on the cursor/handle or other area to select a word.
3. Tapping the handle should show the paste bubble (if there is something in clipboard) *
4. When dragging single handle and then releasing finger, the Paste should appear again. *
5. Dismissing keyboard via long-press-space-bar should also deselect text.
6. Handle touch area should be bigger.
7. Browser address bar:
	1) The original drag-to-select-text behavior should be removed, and keep only drag-to-scroll-text.
	2) "..." should be removed.
	3) Sometimes, long-press-select-word bubble has wrong position.
	4) Sometimes, tap-to-select-all bubble has wrong position.

For next phase:
8. After review, scroll page proposal 2 has some problem if a lot of text is selected. (Please follow Proposal 1.)
9. Long-press on empty space to show only "Select All" (and "Paste" if there is something in clipboard)
10. Paste shortcut won't show automatically after paste once or timeout

Comment 38

[(Inactive after June) George Duan [:gduan] [:喬智]]

(In reply to Omega Feng [:Omega] from comment #37)
> Comment on attachment 8434831 [details] [review]
> PR to master
> 
> Hi George, here are some feedback:
> 
> For current phase:
> 1. Even a text input has no focus, it should still allow
> long-press-to-select behavior.
Event is triggered only when we focus on input. need gecko's help.

> 2. When a text input has focus, it can be either long-press on the
> cursor/handle or other area to select a word.
Gecko should emit textualmenu event.

> 3. Tapping the handle should show the paste bubble (if there is something in
> clipboard) *
bug 1023041

> 4. When dragging single handle and then releasing finger, the Paste should
> appear again. *
bug 1023041

> 5. Dismissing keyboard via long-press-space-bar should also deselect text.
We should file a bug for it.

> 6. Handle touch area should be bigger.
Need gecko's help.

> 7. Browser address bar:
< Browser issues when copy-paste is enabled >
> 	1) The original drag-to-select-text behavior should be removed, and keep
> only drag-to-scroll-text.
> 	2) "..." should be removed.
> 	3) Sometimes, long-press-select-word bubble has wrong position.
> 	4) Sometimes, tap-to-select-all bubble has wrong position.



> For next phase:
> 8. After review, scroll page proposal 2 has some problem if a lot of text is
> selected. (Please follow Proposal 1.)
We should file another bug for it when we decide to follow proposal 1. (need gecko's help)
Put it as next step.

> 9. Long-press on empty space to show only "Select All" (and "Paste" if there
> is something in clipboard)
Same as 8.

> 10. Paste shortcut won't show automatically after paste once or timeout
??

Comment 39

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

(In reply to George Duan [:gduan] [:喬智] from comment #36)
> waiting for result from tbpl gaia-try.

You should paste the Try server URL :)

Comment 40

[(Inactive after June) George Duan [:gduan] [:喬智]]

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=0d853050ab2c

Comment 41

[(Inactive after June) George Duan [:gduan] [:喬智]]

gaia-try and travis pass,
https://tbpl.mozilla.org/?tree=Gaia-Try&rev=484cce2c2caa (B2G Desktop Linux x64 Debug Gu failed for a long time)
https://travis-ci.org/mozilla-b2g/gaia/builds/27198556

merge to master,
https://github.com/mozilla-b2g/gaia/commit/e2d5dcdd0452558456f49e5cb427e0296480f267

Comment 42

[(Inactive after June) George Duan [:gduan] [:喬智]]

Resolved, since gaia work for this step has done.

Hi Howie,
as comment 37 and comment 38, we did found a lot of bugs (mostly need gecko's help) which will affect user experience. Should we file more bugs to trace them?

Comment 43

[howie [:howie]]

Hi George, yes, please fire bugs and put under bug#1023688