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

Status: RESOLVED FIXED

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

Description

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

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 1

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

follow up of bug 938045

Comment 2

[howie [:howie]]

Hi George, can you take this for now? thanks

Comment 3

[(Inactive after June) George Duan [: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?

Comment 4

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

I don't think there is any duplicate bug. And this part is controlled by gecko as George says.

Comment 5

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: 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.

Comment 6

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

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.

Comment 7

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Part2: Generate correct event when dragging touch caret.(carry r+: roc)

Update to address comment.

Comment 8

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Part 1: Rename SetMouseDownState to SetDragState.

Rename SetMouseDownState to SetDragState.

Comment 9

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

try run:  https://tbpl.mozilla.org/?tree=Try&rev=5fe0a5a04552

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/24266caa6b44
https://hg.mozilla.org/integration/mozilla-inbound/rev/feb9b9683e97

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/24266caa6b44
https://hg.mozilla.org/mozilla-central/rev/feb9b9683e97