1269210 - Forward button state not updated on subframe navigation

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jan Henning [:JanH]]

STR:
1. Go to http://www.railfaneurope.net/pix_frameset.html, click on a country flag and go down a few subdirectories.
2. Go back one page.

Expected:
Go forward button becomes enabled.

Observed:
Go forward button remains disabled.
Navigating via the history menu (long pressing the back/forward button) is working normally.

Alternative STR (once bug 1269189 lands):
1. Go to http://www.railfaneurope.net/pix_frameset.html, click on a country flag and go down a few subdirectories.
2. Go back one page.
3. Close the tab and then restore it.
4. Tab is restored with forward button enabled.
5. Go forward.

Expected:
Forward button becomes disabled once you've reached the end of the history stack.

Observed:
Forward button remains enabled.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

afaik, we sync this state with Gecko:
 * Tab.handleLocationChange sets mCanDoBack & mCanDoForward from the Gecko message [1]
 * This sends the tab change event LOCATION_CHANGE [2]
 * BrowserToolbar receives the event and calls, updateNavigationButtons [3] (it also calls this for a few other events like ADDED)

[1]: https://dxr.mozilla.org/mozilla-central/rev/0a25833062a880f369e6f9f622413a94cc671bf4/mobile/android/base/java/org/mozilla/gecko/Tab.java#627
[2]: https://dxr.mozilla.org/mozilla-central/rev/0a25833062a880f369e6f9f622413a94cc671bf4/mobile/android/base/java/org/mozilla/gecko/Tab.java#669
[3]: https://dxr.mozilla.org/mozilla-central/rev/0a25833062a880f369e6f9f622413a94cc671bf4/mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbar.java#479

Comment 2

[Jan Henning [:JanH]]

Attachment: Bug 1269210 - Part 1 - Remove unused variables from location change message and Java tab object.

We used to need these for the back button long press history menu, but now we no longer do.

Review commit: https://reviewboard.mozilla.org/r/111820/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111820/

Comment 3

[Jan Henning [:JanH]]

Attachment: Bug 1269210 - Part 2 - Notify the UI to update the button state on subframe navigation.

Even if we do the rest of our location change processing only for top level location changes, we still need to update the state of the back and forward buttons even on subframe navigation, so they can become enabled/disabled as necessary.

Review commit: https://reviewboard.mozilla.org/r/111822/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111822/

Comment 4

[Andrzej Hunt :ahunt]

Comment on attachment 8836398 [details]
Bug 1269210 - Part 1 - Remove unused variables from location change message and Java tab object.

https://reviewboard.mozilla.org/r/111820/#review113400

Comment 5

[Andrzej Hunt :ahunt]

Comment on attachment 8836399 [details]
Bug 1269210 - Part 2 - Notify the UI to update the button state on subframe navigation.

https://reviewboard.mozilla.org/r/111822/#review113404

Looks good to me!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/391c08abb710
Part 1 - Remove unused variables from location change message and Java tab object. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/fd05b1c1758c
Part 2 - Notify the UI to update the button state on subframe navigation. r=ahunt

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/391c08abb710
https://hg.mozilla.org/mozilla-central/rev/fd05b1c1758c

Comment 8

[Oana Horvath [:ohorvath - out of office]]

Verified as fixed on Nightly 54.0a1(2017-02-16), on Motorola Nexus 6 (Android 7.0) & Huawei Honor 5X (Android 5.1.1).