Closed Bug 1023041 Opened 5 years ago Closed 5 years ago

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

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S2 (15aug)

People

(Reporter: gduan, Assigned: mtseng)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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
Attached patch bug1023041 (obsolete) — Splinter Review
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+
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.
https://hg.mozilla.org/mozilla-central/rev/24266caa6b44
https://hg.mozilla.org/mozilla-central/rev/feb9b9683e97
Status: ASSIGNED → RESOLVED
Closed: 5 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.