Closed Bug 1098161 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Selection, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: pchang, Assigned: pchang)

References

Details

Attachments

(1 file, 5 obsolete files)

Omega, do we need to show the touch caret for this case?
Flags: needinfo?(ofeng)
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.
Flags: needinfo?(ofeng)
(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.
Duplicate of this bug: 1095768
Assignee: nobody → pchang
Priority: -- → P2
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?
(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.
Attachment #8533575 - Flags: review?(roc)
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.
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
Flags: needinfo?(ofeng)
OK, let's show only text dialog(utility bubble) for both long press and short press (paste shortcut) on an empty input.
Flags: needinfo?(ofeng)
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.
For email app, the 'to', 'cc' and 'bcc' are unselectable area. But the subject is content-editable field.
Update patch to support non-editable content with empty text
Attachment #8533575 - Attachment is obsolete: true
Attachment #8537663 - Flags: review?(roc)
Update to select the right non-editable componment.
Attachment #8537663 - Attachment is obsolete: true
Attachment #8537663 - Flags: review?(roc)
Attachment #8537675 - Flags: review?(roc)
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)".
Attachment #8537675 - Flags: review?(roc) → review+
Blocks: 1112473
Attachment #8537675 - Attachment is obsolete: true
Attachment #8539013 - Flags: review+
QA Whiteboard: [textselection]
Depends on: 1110917
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.
(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.
Rebased with bug 110917 and passed local marionette testing.

Wait for try result.
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=3f0a44ece2e7
Attachment #8539013 - Attachment is obsolete: true
Attachment #8544379 - Flags: review+
Testing result is positive. Wait for bug 1110917 to be landed.
Just found this bug doesn't have to depend on bug 1110917. Re-modify the patch and also passed my local marionette testing.
Attachment #8544379 - Attachment is obsolete: true
Attachment #8544945 - Flags: review+
No longer depends on: 1110917
https://hg.mozilla.org/mozilla-central/rev/ba37fdda41fc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
QA Whiteboard: [textselection] → [COM=Text Selection]
You need to log in before you can comment on or make changes to this bug.