Back/forward button state can get out of sync on tablets

RESOLVED FIXED in Firefox 36

Status

()

Firefox for Android
General
P5
normal
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: bnicholson, Assigned: mcomella)

Tracking

(Blocks: 1 bug, {reproducible})

Trunk
Firefox 36
All
Android
reproducible
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox27 affected, firefox28 affected, firefox29 affected, firefox30 affected, fennec+)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

4 years ago
This is a variant of the bug reported by Chenxia at https://bugzilla.mozilla.org/show_bug.cgi?id=917896#c34. Testing these steps on Nightly, we determined bug 917896 was not the cause.

STR:
1) Go to several pages in a row (2+), then click back repeatedly to go back to about:home
2) Repeatedly tap the forward button quickly
3) Go back to about:home again

On about:home, the back button is normally grayed out. After these steps, however, the back button is still active on about:home, and nothing happens when it's clicked. We probably have a race condition somewhere causing the history stack count to get out of sync.
Similarly I notice that we lose forward sometimes when doing:

i) On about:home, visit mozilla.com
ii) Hit back and forward a few times

After a few back and forward navigations you'll be on about:home and the forward button will missing.
Keywords: reproducible
Summary: Back button state can get out of sync on tablets → Back/forward button state can get out of sync on tablets
This is potentially a regression caused by some of the refactorings I landed in 29. But a regression range would be useful here.
tracking-fennec: ? → 29+
Keywords: regressionwindow-wanted

Updated

4 years ago
Assignee: nobody → lucasr.at.mozilla
(Assignee)

Comment 3

4 years ago
In 28 on my Galaxy Tab 3, after navigating to the first page, the back button grayed out so you cannot return to about:home. Hitting the back button closes the browser: related?

Otherwise, I cannot repro (but the device is too slow for me to be sure if it's possible or not).
tracking-fennec: 29+ → ?

Updated

4 years ago
tracking-fennec: ? → 29+

Comment 4

4 years ago
The bug is not a regression, it is reproducible even on 2012 builds.
status-firefox27: ? → affected
status-firefox28: ? → affected
status-firefox30: --- → affected
Keywords: regressionwindow-wanted
Given that this is not a refression, I think we should reconsider the tracking state here. Renominating.
tracking-fennec: 29+ → ?
tracking-fennec: ? → 32+
What can we do to drive this forward?

Comment 7

3 years ago
Looks like this probably isn't going to make 32. Should we re-nom?
Flags: needinfo?(lucasr.at.mozilla)
Yep.
tracking-fennec: 32+ → ?
Flags: needinfo?(lucasr.at.mozilla)
tracking-fennec: ? → +
(Assignee)

Comment 9

3 years ago
Just want to draw attention to this bug.

Lucas, do you want me to take a look?
Flags: needinfo?(lucasr.at.mozilla)
(In reply to Michael Comella (:mcomella) from comment #9)
> Just want to draw attention to this bug.
> 
> Lucas, do you want me to take a look?

Sure, go ahead.
Assignee: lucasr.at.mozilla → michael.l.comella
Flags: needinfo?(lucasr.at.mozilla)
(Assignee)

Comment 11

3 years ago
(Note that there may also be other issues in the code here, but this is just one I found)

It seems the issue is that calling browser.goBack(); twice in succession (e.g. the button is clicked quickly twice [1]) results in two OnHistoryGoBack [2] calls, but only one onLocationChange [3] call and with the first back event's location.

via IRC:

<NeilAway> mcomella: I don't know whether it's Gecko expected behaviour, but I expect all navigation methods to be async except for anchor scrolling, so that if you really want to go back 2 history items you need to use gotoIndex

---

This leaves me with two thoughts on how to fix this:
  1) Queue click events and act after a timeout, using `gotoIndex` to move the appropriate amount of steps back. This is bad because we have to add an artificial wait before responding to user input, and it will break if the user waits long enough and presses back some more.
  2) Wait to receive the onLocationChange call before firing another goBack event. This can be bad because I don't know if waiting for onLocationChange will actually prevent the call from behind hidden and there may be other asynchronous bugs we'd be masking. Additionally, we may also have an artificial delay given we don't know how long it will take to fire onLocationChange. Finally, we don't know that onLocationChange will be called for *all* pages (e.g. about: pages). There is some research to be done here.

It'd also be nice to figure out what desktop does.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=6797921c8f49#1509
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=6797921c8f49#4368
[3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=6797921c8f49#4192
Status: NEW → ASSIGNED
(Assignee)

Comment 12

3 years ago
It seems desktop also uses `browser.goBack()` directy [1] - I wonder what the difference is.

[1]: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js?rev=a7819b6a9411#1669
(Assignee)

Comment 13

3 years ago
Created attachment 8483158 [details] [diff] [review]
(WIP) Fix back button navigation

What do you think of this workaround, Lucas? I actually like the behavior of
this patch because instead of hitting back and being unsure if Android, under
heavy load from your taps, handled your multiple presses correctly (as I
usually feel), it just jumps directly to the page you requested, which seems
cleaner.

For next steps:
  * Fine-tune the delay value (likely a decrease)
  * Combine the back and forward button queues into one object
  * Make the members used by this method "private"
  * Only use the timeout if we know we can go back (or ensure it doesn't matter
if we queue the timeout anyway)
  * How does this interact with page back events
(e.g. `window.history.back()`). Does it matter?

Issues:
  * The browser will only navigate when back is not pressed within 250ms, thus
hitting back forever will cause the browser to never go back. This can be
fixed by immediately initiating navigation when an end index
(e.g. array.length - 1, 0) is reached (though the same race condition could
reappear if both directions are hit, but I expect this use case is unlikely)
  * There is a slight delay before navigating - not much we can do about that
  * The race condition that I believe is causing this issue is only being
covered up with this delay and can presumably be experienced again on slower
devices, or devices under load
Attachment #8483158 - Flags: feedback?(lucasr.at.mozilla)
Comment on attachment 8483158 [details] [diff] [review]
(WIP) Fix back button navigation

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

This really looks like a gecko bug :-/ Consecutive goBack() calls should always lead to the expected behaviour, independently of the async nature of the call under the hood.

The delay approach seems a bit fragile. Maybe you should rely on the history state being committed instead of a delay? i.e. instead of using a delay, use the history listener to figure out whether you have pending events to handle? I'm thinking of something like this:

case "Session:Back":
   if (backCount == 0) {
       browser.goBack()
   }
   backCount++;
   break;

Then in OnHistoryGoBack() you'd do:

OnHistoryGoBack: function(aUri) {
  this._sendHistoryEvent("Back");
  backCount--;
  if (backCount > 0) {
      browser.goBack();
  }
  return true;
}

This way you'd be able to serialize consecutive goBack() calls. The drawback here is a possible delay when quickly tapping on the back button. But at least we wouldn't be adding a delay on the most common interaction (single tap on the back button).

Thoughts?
Attachment #8483158 - Flags: feedback?(lucasr.at.mozilla)
(Assignee)

Comment 15

3 years ago
I tried out the approach in comment 14, but it doesn't fix the bug - the back/forward buttons can still get out of sync.

Any other ideas, Lucas? If you try out the patch I attached, you'll find the behavior of the toolbar is smoothed out and polished. Despite the delay, I think I prefer the feeling. We can always reduce the delay too. (though I guess this would hurt us on the millisecond for millisecond page load tests)
Flags: needinfo?(lucasr.at.mozilla)
Comment on attachment 8483158 [details] [diff] [review]
(WIP) Fix back button navigation

I really don't want to create a queue for BACK or FORWARD buttons.
Clearing NI request based on our discussion on IRC. It seems we want to try throttling the UI instead.
Flags: needinfo?(lucasr.at.mozilla)
(Assignee)

Comment 18

3 years ago
As per IRC, I have disabled the back button until we can re-enable it. Finkle originally mentioned "page load", but I think until location change is enough to fix the asynchronous issue underlying this bug and where I reattached the listener. Note also that this patch only affects the back button's short click.

Disabled, but not appearing disabled: https://people.mozilla.org/~mcomella/apks/mcomella-960746_01.apk
Fully disabled: https://people.mozilla.org/~mcomella/apks/mcomella-960746_02.apk

Lucas, what do you think?

I like the first one, but I'm not sure how to mimic a slow-loading page to see how it affects the feeling.
(Assignee)

Updated

3 years ago
Flags: needinfo?(lucasr.at.mozilla)
(Assignee)

Comment 19

3 years ago
Created attachment 8485789 [details] [diff] [review]
(WIP) Back button disabled, but not visibly

The patch for the first build in comment 18, for any one curious. Notoe that I assume we have to disable both the forward and back buttons when one of them is pressed to fix this bug.
Comment on attachment 8485789 [details] [diff] [review]
(WIP) Back button disabled, but not visibly

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

::: mobile/android/base/toolbar/BrowserToolbar.java
@@ +131,5 @@
>      private ThemedImageButton tabsButton;
>      private ImageButton backButton;
>      private ImageButton forwardButton;
>  
> +    private final View.OnClickListener backButtonOnClickListener;

Maybe import View.OnClickListener directly to make this a little more concise?

@@ +1317,5 @@
>  
>          button.setEnabled(enabled);
>      }
>  
>      public void updateBackButton(Tab tab) {

Does is make sense to restore the back button listener any time updateBackButton is called? Would it be safer to only restore it on LOCATION_CHANGE so that our intent is more obvious here?
Yeah, I prefer the first one UX-wise too. Just wondering about cases where this approach (restoring click listener on locationchange) could backfire and leave the back button disabled forever.
Flags: needinfo?(lucasr.at.mozilla)
(In reply to Lucas Rocha (:lucasr) from comment #20)

> >      public void updateBackButton(Tab tab) {
> 
> Does is make sense to restore the back button listener any time
> updateBackButton is called? Would it be safer to only restore it on
> LOCATION_CHANGE so that our intent is more obvious here?

I assume we call updateBackButton from multiple code paths for a reason. Might have something to do with edge case states where LOCATION_CHANGE is not called correctly, for some reason. I think keeping the change in updateBackButton is probably safest. Let' make sure this change alone does not cause some regressions before trying to optimize it a bit more.
(In reply to Mark Finkle (:mfinkle) from comment #22)
> (In reply to Lucas Rocha (:lucasr) from comment #20)
> 
> > >      public void updateBackButton(Tab tab) {
> > 
> > Does is make sense to restore the back button listener any time
> > updateBackButton is called? Would it be safer to only restore it on
> > LOCATION_CHANGE so that our intent is more obvious here?
> 
> I assume we call updateBackButton from multiple code paths for a reason.
> Might have something to do with edge case states where LOCATION_CHANGE is
> not called correctly, for some reason. I think keeping the change in
> updateBackButton is probably safest. Let' make sure this change alone does
> not cause some regressions before trying to optimize it a bit more.

Careful. The reason updateBackButton() is called from multiple code paths now is not the same than the case we're trying to fix here. To be more specific, this bug might still happen if the START event is triggered before LOCATION_CHANGE (can't remember the expected order of the events now). Please double-check.

I'm fine with going ahead with this approach if there are no obvious regressions. Let's just make sure this new behaviour is well documented for future reference.
(Assignee)

Updated

3 years ago
Blocks: 1065187
(Assignee)

Comment 24

3 years ago
Created attachment 8486870 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

I moved resetting the listeners into the final phase of onTabChanged, which
"Always runs afterwards". [1] It handles the 

Not that while this is no longer relevant, START is called before
LOCATION_CHANGE, seemingly all the time under normal page load.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/BrowserToolbar.java?rev=b28df3d24a87#576
Attachment #8486870 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 25

3 years ago
Comment on attachment 8486870 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

Sorry, just found a bug if the stop button is pressed between navigation button taps.
Attachment #8486870 - Attachment is obsolete: true
Attachment #8486870 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 26

3 years ago
Created attachment 8487602 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

I tried enabling the buttons only on the STOP, SELECTED, and LOCATION_CHANGE
TabEvents but the issue remained (you have to press more sporadically to repro,
which is why I didn't realize this with ealier patches). Waiting for the first
two appears to work, however, which is what this patch does.

STOP indicates the page has finished loading (whether that's by hitting stop or
completed page load) but does not require the page to finish rendering, so
waiting for this event does not feel too terrible, while SELECTED is for when
the current tab changes (including tab addition and removal). However, I think
we should implement the history hint soon, as per bug 1065187.

Note that I'm unsure of what LOAD_ERROR indicates as it does not seem to be
called on 404 and a disabled network - do you know if STOP covers these cases?
Attachment #8487602 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Updated

3 years ago
Attachment #8485789 - Attachment is obsolete: true
(In reply to Michael Comella (:mcomella) from comment #26)

> Note that I'm unsure of what LOAD_ERROR indicates as it does not seem to be
> called on 404 and a disabled network - do you know if STOP covers these
> cases?

LOAD_ERROR is fired for things like trying to navigate to aboot:config (bad scheme) and maybe about:finkle (doesn't exist)
(Assignee)

Comment 28

3 years ago
Created attachment 8487608 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

Should have done more homework: LOAD_ERROR appears to be called when a
JavaScript exception is thrown while loading a URI [1]. Theoretically, this
means STOP does not have to get called, so I added it to the TabEvents that
will enable the navigation buttons.

Also added comments on why I chose to use those particular events.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=bf52063ed95f#969
Attachment #8487608 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 29

3 years ago
For edification:

(In reply to Mark Finkle (:mfinkle) from comment #27)
> LOAD_ERROR is fired for things like trying to navigate to aboot:config (bad
> scheme)

LOAD_ERROR is not fired in this case.

> and maybe about:finkle (doesn't exist)

But LOAD_ERROR is fired here.
> and maybe about:finkle (doesn't exist)

That looks like a bug. Someone should file.
Comment on attachment 8487608 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

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

Looks good. You have rebasing to do now that larch has been merged to m-c :-)

::: mobile/android/base/toolbar/BrowserToolbar.java
@@ +617,5 @@
> +        //
> +        // See `setNavigationButtonListeners` javadoc for more information.
> +        if (msg == TabEvents.STOP ||
> +                msg == TabEvents.SELECTED ||
> +                msg == TabEvents.LOAD_ERROR) {

nit: this indentation looks a bit weird. Maybe align all expressions? Up to you.
Attachment #8487608 - Flags: review?(lucasr.at.mozilla) → review+

Updated

3 years ago
Attachment #8487602 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 32

3 years ago
(In reply to Lucas Rocha (:lucasr) from comment #31)
> ::: mobile/android/base/toolbar/BrowserToolbar.java
> @@ +617,5 @@
> > +        //
> > +        // See `setNavigationButtonListeners` javadoc for more information.
> > +        if (msg == TabEvents.STOP ||
> > +                msg == TabEvents.SELECTED ||
> > +                msg == TabEvents.LOAD_ERROR) {
> 
> nit: this indentation looks a bit weird. Maybe align all expressions? Up to
> you.

I think having the expressions at the same indentation level as the first line in the block is misleading so I'm going to leave it the way it is.
(Assignee)

Comment 33

3 years ago
Created attachment 8488118 [details] [diff] [review]
Disable navigation buttons when they are hit until page load

Rebased - note that I moved the OnClickListeners into classes to keep the constructor short but maintain the final keywords on the members.
(Assignee)

Updated

3 years ago
Attachment #8487602 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8487608 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8483158 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8488118 - Flags: review+
(Assignee)

Comment 34

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/8d663f978a22
(Assignee)

Comment 35

3 years ago
NI: I think this would be good to keep this baking on Nightly because it changes essential UX functionality, but it may be worth uplifting too.
Flags: needinfo?(michael.l.comella)
(Assignee)

Comment 36

3 years ago
(In reply to Richard Newman [:rnewman] from comment #30)
> > and maybe about:finkle (doesn't exist)
> 
> That looks like a bug. Someone should file.

I'm not sure what about this is a bug - Richard, can you explain, or file yourself?
Flags: needinfo?(rnewman)
(In reply to Michael Comella (:mcomella) from comment #36)

> I'm not sure what about this is a bug - Richard, can you explain, or file
> yourself?

Maybe we should show <https://pbs.twimg.com/profile_images/498588007599837184/bpNnvyjB.jpeg>, maybe pulsing the colors between normal and posterized?

mfinkle would probably also enjoy it if we played <https://www.youtube.com/watch?v=xhrBDcQq2DM> in the background.
Flags: needinfo?(rnewman)
(Assignee)

Comment 38

3 years ago
...so did you file or not?
https://hg.mozilla.org/mozilla-central/rev/8d663f978a22
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Backed out for turning bug 941746 into a perma-fail.
https://hg.mozilla.org/mozilla-central/rev/31a43e4f93d2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 35 → ---
(Assignee)

Updated

3 years ago
Blocks: 1068172
(Assignee)

Comment 41

3 years ago
Created attachment 8490410 [details] [diff] [review]
Part 2: Update tests' navigation to wait for back/forward buttons to be re-enabled

The problem with the tests was we need to wait for the back button to be re-enabled before we press it again.

try: https://tbpl.mozilla.org/?tree=Try&rev=a818a08cbd7f
Attachment #8490410 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 42

3 years ago
(In reply to Michael Comella (:mcomella) from comment #35)
> NI: I think this would be good to keep this baking on Nightly because it
> changes essential UX functionality, but it may be worth uplifting too.

Actually, we've had this bug for a while and it's a substantial UX change. As long as it lands before/with the new tablet code, I think it's fine not to uplift.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8490410 [details] [diff] [review]
Part 2: Update tests' navigation to wait for back/forward buttons to be re-enabled

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

Good.

::: mobile/android/base/tests/BaseTest.java
@@ +830,5 @@
> +         * page load completion event before returning.
> +         *
> +         * @param runnable The Runnable to run.
> +         */
> +        private void runAndWaitForPageLoad(final Runnable runnable) {

Shouldn't we update the UITest framework to do the same? At least file a follow-up as reminder.

@@ +841,5 @@
> +            Tabs.registerOnTabsChangedListener(new OnTabsChangedListener() {
> +                @Override
> +                public void onTabChanged(final Tab tab, final TabEvents msg,
> +                        final Object data) {
> +                    if (msg == TabEvents.STOP) {

I assume it's safe to assume the STOP message is on the tab for which the back button was originally pressed?
Attachment #8490410 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 44

3 years ago
(In reply to Lucas Rocha (:lucasr) from comment #43)
> Shouldn't we update the UITest framework to do the same? At least file a
> follow-up as reminder.

bug 1068172.

> @@ +841,5 @@
> I assume it's safe to assume the STOP message is on the tab for which the
> back button was originally pressed?

We should also look for LOAD_ERROR for when an error occurs (and STOP does not appear) - this mirrors the page load events we re-enable the buttons on in BrowserToolbar. 

However, this made me do my homework and I found some issues:
  * Navigating via back and forward over an error page does not show the STOP or LOAD_ERROR messages. The best message we can use is LOADED, but I don't know how that interacts with the other page load events 
  * Long-pressing a button and closing the context menu without selecting an option (i.e. staying on the same page) causes the navigation buttons to remain disabled
  * The clickability of the navigation buttons remains on when the listeners are removed so long-pressing the navigation buttons (without listeners) during page load results in the url bar long press context menu being opened (I'm suprised tapping the back button doesn't open about:home, however)

...I guess I have more work to do.
(Assignee)

Updated

3 years ago
Blocks: 1014156
filter on [mass-p5]
Priority: -- → P5
(Assignee)

Comment 46

3 years ago
After playing around a bit, I find disabling that the back/forward buttons is infuriating on slow loading pages (I used kotaku.com). I've heard issues raised from users in locations where high-speed internet is not available criticize features for working poorly on slow page loads so if this feels bad to me, it will feel much worse to someone else (especially if they don't know what's going on under the hood).

Anthony, what do you think? [1] Would you agree that disabling the navigation buttons after being tapped, until ~page load, isn't really a viable solution?

Instead, I think we should try to fix the issues raised in comment 11 (which may include digging into the platform code), rather than disabling the navigation buttons. This also has the benefit of not being able to put the browser into a strange edge-case state where the navigation buttons get disabled.

[1]: https://people.mozilla.com/~mcomella/apks/mcomella-960746_03.apk
Flags: needinfo?(alam)
Sounds like this is largely a technical issue. But, I talked to Michael and it makes sense to just fix the sync issue as suggested by him.
Flags: needinfo?(alam)
(Assignee)

Comment 48

3 years ago
When gBrowser.goBack() is called twice in quick succession on desktop (e.g. in Scratchpad), the browser will only go back one page. On mobile, only one page is gone back but the back/forward buttons are out of sync - this is because we update the back/forward buttons on the OnHistoryGoBack listener [1][2], where the documentation states [3]:

The listener can prevent any action (except adding a new session history entry) from happening by returning false from the corresponding callback method.

Therefore, these listeners do not guarantee that a back/forward event has been correctly triggered as we see when the back button is hit twice in quick succession and the Android state gets out of sync with Gecko - we should move the messages to Android somewhere that is guaranteed to run.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=d63fe263014c#4489
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tab.java?rev=53d7570e20f2#583
[3]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsISHistoryListener#OnHistoryGoBack()
(Assignee)

Comment 49

3 years ago
Seems desktop does their back/forward button enabling/disabling in onLocationChange [1] - I'll try that out.

[1]: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#4183
(Assignee)

Updated

3 years ago
Attachment #8488118 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8490410 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Blocks: 1097317
(Assignee)

Comment 50

3 years ago
Created attachment 8520980 [details] [diff] [review]
Sync tab state with gecko state in onLocationChange to update navigation buttons

If you *really* abuse the back/forward buttons, you can still get them out of
sync but I think this is a bug in our animations, as opposed to the underlying
platform - normal usage does not seem to cause issues.

Also, we should move the restore zoom functionality to a stable listener, but
in a different bug (see bug 1097317).
Attachment #8520980 - Flags: review?(lucasr.at.mozilla)
(Assignee)

Comment 51

3 years ago
https://tbpl.mozilla.org/?tree=Try&rev=5c522df50776
Comment on attachment 8520980 [details] [diff] [review]
Sync tab state with gecko state in onLocationChange to update navigation buttons

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

I like how simple this looks. Let's see it goes.
Attachment #8520980 - Flags: review?(lucasr.at.mozilla) → review+
(Assignee)

Comment 53

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/b83cdceb5b0c
https://hg.mozilla.org/mozilla-central/rev/b83cdceb5b0c
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36

Updated

3 years ago
Depends on: 1124190
You need to log in before you can comment on or make changes to this bug.