1098161 - [Text Selection]Trigger text dialog by long press when input filed is empty

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P2)

Description

[Peter Chang[:pchang]]

This bug is the follow up bug from bug 1059165 comment 35.
https://bugzilla.mozilla.org/show_bug.cgi?id=1059165#c35

Comment 1

[Peter Chang[:pchang]]

Omega, do we need to show the touch caret for this case?

Comment 2

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

Basically I accept either showing the touch caret or not, but personally prefer showing the caret. In other cases, when the utility bubble shows, the touch carets are always visible. So, it's better to keep some consistency here.

Comment 3

[Peter Chang[:pchang]]

(In reply to Omega Feng [:Omega] [:馮於懋] (please ni?) from comment #2)
> Basically I accept either showing the touch caret or not, but personally
> prefer showing the caret. In other cases, when the utility bubble shows, the
> touch carets are always visible. So, it's better to keep some consistency
> here.

Thanks, I also prefer to show touch caret during this use case. I will check the technical part based on our above discussion.

Comment 5

[Steve Chung [:steveck]]

Hi Peter, I saw some weird result while long press on empty contentEditable element in message app:
- Long press on message app recipient field
- Long press on message app subject field

In this 2 cases the caret will be in wrong position, will you also solve in this bug?

Comment 6

[Peter Chang[:pchang]]

(In reply to Steve Chung [:steveck] from comment #5)
> Hi Peter, I saw some weird result while long press on empty contentEditable
> element in message app:
> - Long press on message app recipient field
> - Long press on message app subject field
> 
> In this 2 cases the caret will be in wrong position, will you also solve in
> this bug?

Yes, I will fix them too.

Comment 7

[Peter Chang[:pchang]]

Attachment: Display touch caret and text dialog when long press on a empty content

Comment 8

[Peter Chang[:pchang]]

Cancel the review because I would like to check with UX about the visibility of touch caret for long tap and short cut mode for empty content.

Comment 9

[Peter Chang[:pchang]]

Omega, this bug is trying to implement the following UX spec[1] for long tap use case.
Based on UX spec, we would display touch caret and text dialog(utility bubble) when long press on empty content.

But it only shows text dialog for empty content area when there is something copied within 15 seconds(short-cut mode). Is it possible to display text dialog for empty content field no matter long press or short cut?

[1]https://bug921965.bugzilla.mozilla.org/attachment.cgi?id=8521959#9

Comment 10

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

OK, let's show only text dialog(utility bubble) for both long press and short press (paste shortcut) on an empty input.

Comment 11

[Peter Chang[:pchang]]

Based on comment 10 to re-write the patch.
But I found a problem after non-editable is enabled.

The 'to' field of message app was not content editable, need to provide solution for non-editable content.

Comment 12

[Peter Chang[:pchang]]

For email app, the 'to', 'cc' and 'bcc' are unselectable area. But the subject is content-editable field.

Comment 13

[Peter Chang[:pchang]]

Attachment: v2 Display text dialog when long press on the content with empty text

Update patch to support non-editable content with empty text

Comment 14

[Peter Chang[:pchang]]

Attachment: v3 Display text dialog when long press on the content with empty text

Update to select the right non-editable componment.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8537675 [details] [diff] [review]
v3 Display text dialog when long press on the content with empty text

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

::: layout/base/SelectionCarets.cpp
@@ +613,5 @@
> +      // Long tap event on this non-editable content with empty text, no action
> +      // for selectioncarets but need to dispatch the touchcarettap event
> +      // to support the short cut mode
> +      DispatchCustomEvent(NS_LITERAL_STRING("touchcarettap"));
> +      return NS_OK;

We should only have one copy of this code block. Move selectedContent out and put this code block after "if (editingHost)".

Comment 16

[Peter Chang[:pchang]]

Attachment: v4 Display text dialog when long press on the content with empty text

Comment 17

[Peter Chang[:pchang]]

Try server result is positive.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fb9ce30f125

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4aad6ecf4b7d

Comment 19

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=4862960&repo=mozilla-inbound

Comment 20

[Peter Chang[:pchang]]

The attachment 8539013 [details] [diff] [review] is also trying to change the flow for content non-editable and therefore, it causes the marionette failed. With the bug 1110917, it starts to check the content is focusable or non-focusable and we should check the content is empty when this content is focusable.

Comment 21

[Peter Chang[:pchang]]

(In reply to peter chang[:pchang][:peter] from comment #20)
> The attachment 8539013 [details] [diff] [review] is also trying to change
> the flow for content non-editable and therefore, it causes the marionette
> failed. With the bug 1110917, it starts to check the content is focusable or
> non-focusable and we should check the content is empty when this content is
> focusable.

After applying bug 1110917, all marionette test for selectioncarets pass but only the testing items about input element are failed. Looks like it has problem to detect the text length for input element.

Comment 22

[Peter Chang[:pchang]]

Attachment: v5 Display text dialog when long press on the content with empty text

Rebased with bug 110917 and passed local marionette testing.

Wait for try result.
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3f0a44ece2e7

Comment 23

[Peter Chang[:pchang]]

Testing result is positive. Wait for bug 1110917 to be landed.

Comment 24

[Peter Chang[:pchang]]

Attachment: v6 Display text dialog when long press on the content with empty text

Just found this bug doesn't have to depend on bug 1110917. Re-modify the patch and also passed my local marionette testing.

Comment 25

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7e2a5ea5a29

The v6 patch passed the testing.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba37fdda41fc

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ba37fdda41fc