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)
Tracking
()
NEW
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
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(tlin)
Comment 1•9 years ago
|
||
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)
Reporter | ||
Comment 2•9 years ago
|
||
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.
Comment 3•9 years ago
|
||
(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)
Comment 4•9 years ago
|
||
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)
Reporter | ||
Comment 5•9 years ago
|
||
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. :-)
Reporter | ||
Comment 6•9 years ago
|
||
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 7•9 years ago
|
||
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)
Comment 8•9 years ago
|
||
The blur event handling in AccessibleCaret does not change much. It also has this issue like SelectionCarets.
Blocks: AccessibleCaret
Summary: Blur event closes SelectionCarets but leaves visible selection on page → Blur event closes AccessibleCaret but leaves visible selection on page
Comment 9•7 years ago
|
||
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 hidden (mozreview-request) |
Comment 11•7 years ago
|
||
mozreview-review |
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+
Comment 12•7 years ago
|
||
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/079ee7b41c3a Update carets when document becomes visible. r=mtseng
Comment 13•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/7d0ff0aae0a0 for reftest failures: https://treeherder.mozilla.org/logviewer.html#?job_id=7549785&repo=autoland https://treeherder.mozilla.org/logviewer.html#?job_id=7549823&repo=autoland https://treeherder.mozilla.org/logviewer.html#?job_id=7549749&repo=autoland https://treeherder.mozilla.org/logviewer.html#?job_id=7549740&repo=autoland https://treeherder.mozilla.org/logviewer.html#?job_id=7549816&repo=autoland
Updated•7 years ago
|
Priority: -- → P3
Comment 14•7 years ago
|
||
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
Updated•7 years ago
|
Attachment #8595190 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8814805 -
Attachment is obsolete: true
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•