1114853 - [Text Selection] when calling input.focus(), the text selection bubble would show up randomly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P2)

Description

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

When writing patch for bug 1103809, I noticed that if we focus on input programmatically, the text selection bubble would show up randomly without any reason.

[STR]

With bug 1103809 applied first.

1. Go to settings app > call settings.
2. click on voicemail menuItem.
3. click back button and go back to previous panel.
4. repeat step 2 & step 3 and you will see this inconsistent behavior.

[Expected Result]

Text selection bubble should show up correctly no matter how.

[Actual Result]

Text selection bubble would show up randomly.

Comment 1

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

Hi Morris,
I found that, if js call selection related api, such as input.focus + input.setSelectionRange / input.select, the bubble will show up, however, it would not disappear if I click other place with user-select: none, there would be no selectionstatechanged event pop out from gecko.
Only if I click one of the buttons, it would become normal again.

could you kindly help to check this issue?

Comment 2

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

Attachment: Part 1: Change rule of forcing dispatch selection state changed event.

Comment 3

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

Attachment: Part 2: Remove useless SetVisibility calls.

Comment 4

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

Attachment: Part 3: Reset mSelectionVisibleInScrollFrames when visibility set to false.

Comment 5

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

Attachment: Part 4: Bail out if selections are mismatch.

Comment 6

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

Attachment: Part 5: Don't dispatch event when we're at drag mode.

Comment 7

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

Attachment: (gaia-part)Remove and adjust some useless logic.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8541127 [details] [diff] [review]
Part 3: Reset mSelectionVisibleInScrollFrames when visibility set to false.

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

Thanks for fixing this variable which I added.

::: layout/base/SelectionCarets.cpp
@@ +293,5 @@
>                          (aVisible ? "shown" : "hidden"));
>      return;
>    }
>  
> +  if (aVisible == false) {

Nit: if (!aVisible)

Comment 9

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8541128 [details] [diff] [review]
Part 4: Bail out if selections are mismatch.

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

Nice! nsCaret and TouchCaret already did this.

Comment 10

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

Attachment: Part 3: Reset mSelectionVisibleInScrollFrames when visibility set to false v2.

fix nit.

Comment 11

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

Comment on attachment 8541125 [details] [diff] [review]
Part 1: Change rule of forcing dispatch selection state changed event.

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

::: dom/browser-element/BrowserElementChildPreload.js
@@ +625,5 @@
>      }
>  
> +    // If we select something and selection range is visible, we set the
> +    // forceDispatchSelectionStateChanged flag as true to dispatch the
> +    // next SelecitonChange event so that the parent side can

SelectionChange

Comment 12

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

Attachment: Part 1: Change rule of forcing dispatch selection state changed event v2. (carry r+: roc)

Update to addressed comment

Comment 13

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

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

Comment 14

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

Attachment: Part 4: Bail out if selections are mismatch v2. (carry r+: roc)

Fix test fail.

Comment 15

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

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

Comment 16

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

Try is good.

Comment 17

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/33293793d8b6
https://hg.mozilla.org/mozilla-central/rev/dc9a5477f42f
https://hg.mozilla.org/mozilla-central/rev/8576d2d7cdfb
https://hg.mozilla.org/mozilla-central/rev/0b2e3bcfe0d5
https://hg.mozilla.org/mozilla-central/rev/7200cfff578b

Comment 18

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

Attachment: PR to gaia master

Hi Alive,
could I have your review for this patch?
Thanks.

Comment 19

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

Comment on attachment 8543841 [details] [review]
PR to gaia master

put gaia patch in bug 1110963.