User Agent: Mozilla/5.0 (Android 7.0; Mobile; rv:49.0) Gecko/49.0 Firefox/49.0 Build ID: 20160916104719 Steps to reproduce: When I upgrade my Nexus 6P to Android 7.0 Nougat, long pressing Android system 'back' button does not result in showing history of the tab after browsing a few web pages. When I did long pressing 'back' icon available on firefox menu on the top-left, it worked as expected even on Android 7.0 Nougat. When on Android 6.0 Marshmallow, Actual results: Long pressing Android system 'back' button results in nothing. Expected results: Long pressing Android system 'back' button results in showing history of the tab.

I can reproduce this on a Nexus 6P running Android 7.0. This is not listed in the behavior changes of Android 7.0 [1]. Ahunt, can you investigate? I guess we are handling this here: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#3835 [1] https://developer.android.com/about/versions/nougat/android-7.0-changes.html

We don't seem to be receiving onKeyLongPress at all - I'll have a dig in the Android sources to see if something has changed to confirm that this is in fact an Android change (which seems likely seeing as this appeared with 7.0). I think we could probably use onKeyUp() with event.getDownTime() instead, so this isn't too catastrophic.

(In reply to Andrzej Hunt :ahunt from comment #2) > We don't seem to be receiving onKeyLongPress at all - I'll have a dig in the > Android sources to see if something has changed to confirm that this is in > fact an Android change (which seems likely seeing as this appeared with 7.0). > > I think we could probably use onKeyUp() with event.getDownTime() instead, so > this isn't too catastrophic. Android bug report at: https://code.google.com/p/android/issues/detail?id=213738

(In reply to Andrzej Hunt :ahunt from comment #3) > Android bug report at: > https://code.google.com/p/android/issues/detail?id=213738 Argh. (In reply to Andrzej Hunt :ahunt from comment #2) > I think we could probably use onKeyUp() with event.getDownTime() instead, so > this isn't too catastrophic. Do we get that for back?

(In reply to Sebastian Kaspari (:sebastian) from comment #4) > > Do we get that for back? Fortunately yes (at least on my N6P). I did dig into this yesterday: TL;DR version: yes, back button handling has changed, so no apps will receive long-press events. Long version: It seems Android has changed the way long-presses are handled for system buttons: this looks like an unintentional oversight (possibly related to support for having multiple navigation bars on screen? I have no idea...). The button-handling is in KeyButtonView, which AFAICT hasn't changed significantly: http://androidxref.com/7.0.0_r1/xref/frameworks/base/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyButtonView.java The key issue is at line 64: previously isLongClickable() was false (this is a property of View), so the second branch of the if/else is run, and a LONG_PRESS is emitted. This seems to not be broken. In 7.0, some code was added to add a longClickListener to the back button, which causes KeyButtonView to no longer emit the long-press even (isLongClickable() returns true once there's a longClickListener). Then I discovered that there's a new listener which always receives the long-press, but that this is only used for a "lock task mode" in the (also new) PhoneStatusBar: http://androidxref.com/7.0.0_r1/xref/frameworks/base/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java#handleLongPressBack This lock task mode doesn't seem to be new, but the way the back button is handled is new (I'm guessing in 6.0 and earlier, either the long-press to exit lock-task either didn't exist, or the long-press listener was added only if lock-task mode is running - whereas now the listener is always added). More on lock-task: https://developer.android.com/work/cosu.html

Comment on attachment 8795374 [details] Bug 1304688 - Pre: add Versions.preN to AppConstants https://reviewboard.mozilla.org/r/81444/#review80776

Comment on attachment 8795375 [details] Bug 1304688 - Implement long-press timer to workaround broken onKeyLongPress on Android N https://reviewboard.mozilla.org/r/81446/#review80960 ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:551 (Diff revision 1) > + private Runnable mCheckLongPress; > + { > + if (!Versions.preN) { > + mCheckLongPress = new Runnable() { > + public void run() { > + handleBackLongPress(); > + } > + }; > + } > + } This syntax is a bit confusing at first. :) ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3863 (Diff revision 1) > /** > - * This will detect if the key pressed is back. If so, will show the history. > + * Handle a long press on the back button > + * @return <code>null</code> if the press wasn't handled, otherwise a Boolean corresponding to > + * the return value of KeyEvent.Callback.onKeyLongPress > */ > - @Override > + private Boolean handleBackLongPress() { Do we really need a Boolean object here? Shouldn't be true/false for handled / not handled be enough? ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3867 (Diff revision 1) > - public boolean onKeyLongPress(int keyCode, KeyEvent event) { > - if (keyCode == KeyEvent.KEYCODE_BACK) { > - // If the tab search history is already shown, do nothing. > + // If the tab search history is already shown, do nothing. > - TabHistoryFragment frag = (TabHistoryFragment) getSupportFragmentManager().findFragmentByTag(TAB_HISTORY_FRAGMENT_TAG); > + TabHistoryFragment frag = (TabHistoryFragment) getSupportFragmentManager().findFragmentByTag(TAB_HISTORY_FRAGMENT_TAG); > - if (frag != null) { > + if (frag != null) { > - return false; > + return true; Why is this now 'true' instead of 'false'?

(In reply to Sebastian Kaspari (:sebastian) from comment #9) > > Do we really need a Boolean object here? Shouldn't be true/false for handled > / not handled be enough? Yup, makes sense. I got confused by the way the original code work, and ended up overcomplicating things... > > ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:3867 > (Diff revision 1) > > - public boolean onKeyLongPress(int keyCode, KeyEvent event) { > > - if (keyCode == KeyEvent.KEYCODE_BACK) { > > - // If the tab search history is already shown, do nothing. > > + // If the tab search history is already shown, do nothing. > > - TabHistoryFragment frag = (TabHistoryFragment) getSupportFragmentManager().findFragmentByTag(TAB_HISTORY_FRAGMENT_TAG); > > + TabHistoryFragment frag = (TabHistoryFragment) getSupportFragmentManager().findFragmentByTag(TAB_HISTORY_FRAGMENT_TAG); > > - if (frag != null) { > > + if (frag != null) { > > - return false; > > + return true; > > Why is this now 'true' instead of 'false'? We're supposed to return true if we've handled the event - in this case we specifically seem to want to ignore the long-press since we're showing the history list. It seemed more sane to return true (we've handled the event by ignoring it), instead of false (we've ignored the event by not handling it, but anyone else could handle it). Either way, I don't think this has any real effect since no one else handles the long-press, maybe I should just leave the code as is if it's already working.

Pushed by ahunt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/11cf2ce8a9bd Pre: add Versions.preN to AppConstants r=sebastian https://hg.mozilla.org/integration/autoland/rev/8d8964596de0 Implement long-press timer to workaround broken onKeyLongPress on Android N r=sebastian

https://hg.mozilla.org/mozilla-central/rev/11cf2ce8a9bd https://hg.mozilla.org/mozilla-central/rev/8d8964596de0

After visiting a few pages, long tapping on the android back button, will display a history list, so: Verified as fixed using: Device: Nexus 6P (Android 7.0) Build: Firefox for Android 52.0a1 (2016-10-02)

Comment on attachment 8795374 [details] Bug 1304688 - Pre: add Versions.preN to AppConstants Approval Request Comment [Feature/regressing bug #]: Android Bug 213738 - https://code.google.com/p/android/issues/detail?id=213738 [User impact if declined]: Can't view history by long-pressing back button on devices running Android 7. [Describe test coverage new/current, TreeHerder]: manual testing, tested on nightly. [Risks and why]: This patch: low-risk, only adds a version constant. (Second patch: low to medium risk: we've added a timer that is only run in limited circumstances, this code is based on Android's own long-press code that is already well tested.) [String/UUID change made/needed]: none.

Comment on attachment 8795375 [details] Bug 1304688 - Implement long-press timer to workaround broken onKeyLongPress on Android N Approval Request Comment [Feature/regressing bug #]: Android Bug 213738 - https://code.google.com/p/android/issues/detail?id=213738 [User impact if declined]: Can't view history by long-pressing back button on devices running Android 7. [Describe test coverage new/current, TreeHerder]: manual testing, tested on nightly. [Risks and why]: This patch: low to medium risk: we've added a timer that is only run in limited circumstances, this code is based on Android's own long-press code that is already well tested. (First patch: low-risk, only adds a version constant.) [String/UUID change made/needed]: none.

Comment on attachment 8795374 [details] Bug 1304688 - Pre: add Versions.preN to AppConstants Fix was verified on Nightly52, Aurora51+, Beta50+

https://hg.mozilla.org/releases/mozilla-aurora/rev/ac5b00f946f4 https://hg.mozilla.org/releases/mozilla-aurora/rev/8096f65a788a

https://hg.mozilla.org/releases/mozilla-beta/rev/bb2c7737ea03 https://hg.mozilla.org/releases/mozilla-beta/rev/437d50a541e2

Verified as fixed on Nexus 6P (Android 7.0) on Fennec 50 Beta 4 and latest Aurora.

My device has just updated from android 6 to Nougat 7.0 and Im running Firefox 56.0. This problem now exists for me as reported. Maybe this patch never made it to v56? Is there a way of raising this again or is this comment sufficient to raise the profile/review?