2018858 - Move performHapicFeedback to nsIWidget

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[Makoto Kato [:m_kato]]

By bug 1817606, we use View.performHapticFeedback instead of vibrator API. But when I investigate other platform (UIKit), UIKit's API (UISelectionFeedbackGenerator) also needs UIView. So I think that we should move performHapicFeedback from own service to nsIWidget method, then get a rid of nsIHapicFeedback.

Comment 1

[LIU Xinyu [:imnotlxy]]

I can take this if you agree :-)

Comment 2

[Makoto Kato [:m_kato]]

(In reply to LIU Xinyu [:imnotlxy] from comment #1)

I can take this if you agree :-)

Yes.

Comment 3

[LIU Xinyu [:imnotlxy]]

I've thought a bit and don't think it's a good idea.

On Android, almost all Views can perform haptic feedback, so adding a performHapticFeedback method to the base class View is resonable. Unfortunately it's not the case for nsIWidget - only nsWindow on Android (or UIKit in the future) can perform haptic feedback. I don't think we should add such a method to nsIWidget only for two subclasses.

Also, IPC is necessary. No haptic feedback can be performed without going through HAL. Thus hal::PerformHapticFeedback should be kept.

Anyway, I agree that nsIHapicFeedback is no longer necessary. I suggest we remove it and AccessibleCaretManager call hal::PerformHapticFeedback directly.

Comment 4

[Makoto Kato [:m_kato]]

Since hapic feedback depends on nsIWidget / nsWindow, it isn't good if this service is in hal now. If this feature depends on window (Android and UIkit), it is better to add a IPC method in PBrowser, instead of hal. Accessible Caret has nsPresShell, so it can access nsIWidget (PuppetWidget in child process) directly.

Currently, GeckoView doesn't support multiple screens and multiple windows in multiple screens. At the future, Android might have multiple windows like desktop. Also, the window that shows accessible caret doesn't assures focused window, such as a race condition, or browser side implementation. So I think that is better to add nsIWidget::PerformHapticFeedback.

Comment 5

[LIU Xinyu [:imnotlxy]]

Attachment: Bug 2018858 - Move PerformHapicFeedback to nsIWidget

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b99253dfd8cd
https://hg.mozilla.org/integration/autoland/rev/a33d3d1ed239
Move PerformHapicFeedback to nsIWidget r=geckoview-reviewers,layout-reviewers,emilio,m_kato

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/a33d3d1ed239