[Window Management] TextSelectionDiag - after moving single caret , the menu won't show again

RESOLVED FIXED in 2.1 S2 (15aug)

Status

Firefox OS
Gaia::System::Window Mgmt
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gduan, Assigned: mtseng)

Tracking

unspecified
2.1 S2 (15aug)
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

1. copy or cut any text and touch any place of content, only one caret displaying
2. moving the caret, the dialog disappear
3. stop moving, user try to open the menu again by clicking it. --- failed

User should be able to relaunch the menu by clicking caret.

Comment 2

4 years ago
Hi George, can you take this for now? thanks
Flags: needinfo?(gduan)
Hi Howie,

gecko controls most part of caret's behavior, so does this one.

Hi Morris,

We have offline discussed it before. Is there any duplicate bug?
Flags: needinfo?(gduan) → needinfo?(mtseng)
I don't think there is any duplicate bug. And this part is controlled by gecko as George says.
Flags: needinfo?(mtseng)
Depends on: 1020802
Created attachment 8466003 [details] [diff] [review]
bug1023041

UX spec shows we should close utility bubble when user dragging caret and show the bubble when user releasing the caret.

The modification of this patch is firing selection change event with drag/mouseup reason when dragging/releasing touch caret. So we can close the bubble when we got drag reason and show the bubble when we got mouseup reason.

BTW, Closing bubble when got drag reason is handled by bug 1020802.
Assignee: nobody → mtseng
Status: NEW → ASSIGNED
Attachment #8466003 - Flags: review?(roc)
Comment on attachment 8466003 [details] [diff] [review]
bug1023041

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

::: layout/base/TouchCaret.cpp
@@ +477,5 @@
> +  nsISelection* caretSelection = caret->GetCaretDOMSelection();
> +  nsRect focusRect;
> +  nsIFrame* caretFocusFrame = caret->GetGeometry(caretSelection, &focusRect);
> +  nsRefPtr<nsFrameSelection> fs = caretFocusFrame->GetFrameSelection();
> +  if (fs->GetMouseDownState() == aState) {

Don't do this check. nsFrameSelection::SetMouseDownState does it for you.

::: layout/base/TouchCaret.h
@@ +154,5 @@
> +   * caret. The reason for setting this state is it will fire drag reason
> +   * when moving caret and fire mouseup reason when releasing caret. So that
> +   * the display behavior of copy/paste menu becomes more reasonable.
> +   */
> +  void SetMouseDownState(bool aState);

Call this SetSelectionDragState instead since we're not really dealing with mouse events here.

If you could rename the functionality in nsFrameSelection and related classes to SetDragState etc, that would be great --- as a separate patch.
Attachment #8466003 - Flags: review?(roc) → review+
Created attachment 8466901 [details] [diff] [review]
Part2: Generate correct event when dragging touch caret.(carry r+: roc)

Update to address comment.
Attachment #8466003 - Attachment is obsolete: true
Created attachment 8466902 [details] [diff] [review]
Part 1: Rename SetMouseDownState to SetDragState.

Rename SetMouseDownState to SetDragState.
Attachment #8466902 - Flags: review?(roc)
Attachment #8466901 - Attachment description: bug1023041-p2 (carry r+: roc) → Part2: Generate correct event when dragging touch caret.(carry r+: roc)
Attachment #8466902 - Attachment description: bug1023041-p1 → Part 1: Rename SetMouseDownState to SetDragState.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/24266caa6b44
https://hg.mozilla.org/mozilla-central/rev/feb9b9683e97
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S2 (15aug)
You need to log in before you can comment on or make changes to this bug.