Open Bug 1156160 Opened 9 years ago Updated 2 years ago

Blur event closes AccessibleCaret but leaves visible selection on page

Categories

(Core :: DOM: Selection, defect, P3)

ARM
Android
defect

Tracking

()

People

(Reporter: capella, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 obsolete files)

In the video [0] where I'm navigated to page [1] I've long-tapped to select a word and display the SelectionCarets and the Android ActionBar.

If I tap the image above it, focus manager [2] triggers SelectionCarets::NotifyBlur [3] and forces the SelectionCaret to Visibility(false) and hides the ActionBar, but leaving the selection itself visible as an apparent artifact.

I'm not sure how this works on B2G with the Utility Bubble (no device/setup to test), but in the new Android implementation, it feels like a bug. ActionBar visibility is based on Carets visibility, and w/o Carets/ActionBar, there's nothing useful the user can do with the selected text.

I wonder if SelectionCarets::NotifyBlur() should:
   a) UpdateSelectionCarets() instead to conditionally close SelectionCarets if selection persists?
   b) RemoveAllRanges() to clear the "artifact"?
or c) A blend of a) and b) based on whether we're a B2G or Android build?


[0] https://www.dropbox.com/s/4vt9t2tqadnzck6/focusMgrBlurBug.mp4?dl=0
[1] http://arstechnica.com/science/2015/04/new-evidence-that-dark-matter-could-be-self-interacting/
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp?rev=1cde9e7658e5&mark=1692-1694#1685
[3] http://mxr.mozilla.org/mozilla-central/source/layout/base/SelectionCarets.cpp?rev=b1f4b4120e21&mark=1101-1101#1097
Flags: needinfo?(tlin)
I don't know why the selection retains after tapping on the image while it is cleared after tapping on other text. BTW, Google chrome behaves the same though.

On B2G, the utility bubble is a global object across all documents and all apps, which prolongs to stay on the screen if we did not send a mozselectionstatechanged event to tell it to close while the document is blurred.

Solution a) might work for current b2g if we carefully choose when to dispatch mozselectionstatechanged.

Solution b) is not preferred since it makes the core selection behaves different when SelectionCarets is enabled.

Morris, how about we hide selection-carets and dispatch mozselectionstatechanged event only if the blur event is leaving document? Could we have a better handling on the refactored touch/selection-carets?
Flags: needinfo?(tlin) → needinfo?(mtseng)
Good idea! I didn't think to check Google Chrome, but now I see that yes, it behaves the same, in that it leaves the selection visible.

But, it also leaves visible the SelectionCarets, which we currently don't do. So I'd think (a) is the right solution, which would work for Android, if it's also acceptable for B2G.
(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #1)
> I don't know why the selection retains after tapping on the image while it
> is cleared after tapping on other text. BTW, Google chrome behaves the same
> though.
> 
> On B2G, the utility bubble is a global object across all documents and all
> apps, which prolongs to stay on the screen if we did not send a
> mozselectionstatechanged event to tell it to close while the document is
> blurred.
> 
> Solution a) might work for current b2g if we carefully choose when to
> dispatch mozselectionstatechanged.
> 
> Solution b) is not preferred since it makes the core selection behaves
> different when SelectionCarets is enabled.
> 
> Morris, how about we hide selection-carets and dispatch
> mozselectionstatechanged event only if the blur event is leaving document?
> Could we have a better handling on the refactored touch/selection-carets?
I think we should categorize the reason that causes the blur. If you saw nsFocusManager::Blur, there are  complex logics. So the right thing is, we should know what is the reason for blur, and choose correct behavior for this.

I don't think we have better handling on the refactored works now.
Flags: needinfo?(mtseng)
On B2G, suppose a page is blurred with an active selection. When the
page is focused again, we'll have a visible selection without
selection-carets and utility bubble. This is not useful. The only way to
re-enable selection-carets and utility bubble is to make another
selection.

After this patch, we will not hide selection-carets when receiving blur
events. Selection-carets persist when the document is focused. Then we
can tap any selection-carets to bring the utility bubble back. This
should make the copy paste system easier to use.
Attachment #8595190 - Flags: review?(mtseng)
Avoiding the blur in all cases will regress on both B2G and Android.

If you're leaving the document on a tab switch, we need the selectionCarets visibility to be hidden, (even if you leave the text selected), as we close our ActionBar when the blue carets hide. The visibility of the ActionBar and the carets are usually linked in Android, at least for FF Phone, and Chrome's implementation.

Something like the below works better for us.
-SetVisibility(false);
 if (aIsLeavingDocument) {
+  SetVisibility(false);
   CancelLongTapDetector();
 }


There is still an immediate regression for B2G however with this patch, as there are times you will still need to hide the VisibilityCarets when |aIsLeavingDocument| is *not* true, such as during Navigation forward/backward, and shown in this example:

a) If you visit the page [1]
b) Long tap the word "when" in the text to display the selection carets
c) Tap the image to navigate
d) Observe the carets visible incorrectly [2]

[1] http://en.m.wikipedia.org/wiki/Tidal_stripping
[2] https://www.dropbox.com/s/rodnkuzptqhngaf/bug1156160_carets.png?dl=0


I'm not really in a hurry for this on the Android side. If you don't mind, I'd like to suggest we both look here a little deeper. :-)
Oh, heh - Chrome seems to have a problem with that last bit also!

https://www.dropbox.com/s/eidguw73iw1189t/bug1156160_caretsChrome.png?dl=0
Comment on attachment 8595190 [details] [diff] [review]
Do not hide selection-carets when receiving blur events. (v1)

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

Mark, you're right. I did not consider the wikipedia case in comment 5. We should investigate further.
Attachment #8595190 - Flags: review?(mtseng)
The blur event handling in AccessibleCaret does not change much. It also has this issue like SelectionCarets.
Summary: Blur event closes SelectionCarets but leaves visible selection on page → Blur event closes AccessibleCaret but leaves visible selection on page
In most of the scenario, the blur event happen when the user switch between tabs. We could call UpdateCarets when the document becomes foreground so that we won't have a selection without carets escort.
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Comment on attachment 8814805 [details]
Bug 1156160 - Update carets when document becomes visible.

https://reviewboard.mozilla.org/r/95900/#review96962
Attachment #8814805 - Flags: review?(mtseng) → review+
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/079ee7b41c3a
Update carets when document becomes visible. r=mtseng
Priority: -- → P3
Besides the reftest failures, the solution uploaded in comment 10 is also incomplete. It only brings back the carets for tab switching. On desktop, if the user switches to other program and then back to Firefox, the carets are not shown on the visible selection.

Given above, I won't continue to fix this bug until a better approach jumps into my mind.
Assignee: tlin → nobody
Status: ASSIGNED → NEW
Attachment #8595190 - Attachment is obsolete: true
Attachment #8814805 - Attachment is obsolete: true
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: