859391 - Packaged apps using pushState/replaceState generate 404s

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)

Description

[Matt Basta [:basta]]

A packaged app that uses pushState/replaceState to manage its navigation state is inherently broken. Consider the following scenario:

1. User clicks link in packaged app in /index.html
2. Click event is handled by app: preventDefault() is called and pushState() is called with a URL of /foo/bar
3. User switches away from the app, page is unloaded from memory
4. User returns to the app

In this scenario, the current behavior is that Firefox will try to load /foo/bar inside the package, which obviously does not exist. A "404" is shown saying something like:

    Firefox can't find the file at app://{guid}/foo/bar.

Instead, Firefox should do one of the following:

1. Load /index.html (the app's launch_path) with window.location.* set to the appropriate values.
2. Ignore the app's previous state and start at the app's launch path.
3. Keep a saved copy of the app's state on the device and load that state into memory (rather than simply keeping the app's current URL). This is what Android does for native Android apps.

This does not appear to be a bug on FXOS since FXOS observes the third behavior above (correct me if I'm wrong).

Comment 1

[Jason Smith [:jsmith]]

Sounds like a cut and dry P1 A4A to me.

Comment 2

[Matt Basta [:basta]]

This currently means that testing a packaged app on Android means that the app is dead after the first use. After "closing" the app, it goes into the broken state described in comment 0. The best fix that I've come up with is to uninstall the app and reinstall it.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - Didn't you have a patch for this?

Comment 4

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Matt Basta [:basta] from comment #2)
> This currently means that testing a packaged app on Android means that the
> app is dead after the first use. After "closing" the app, it goes into the
> broken state described in comment 0. The best fix that I've come up with is
> to uninstall the app and reinstall it.

Matt, can you provide a URL for the packaged app you used to reproduce this problem?

Comment 5

[Matt Basta [:basta]]

The problem has been reproducible for me with the Nightly build of the marketplace:

http://inferno.paas.allizom.org/

Comment 6

[Jason Smith [:jsmith]]

Explanation is needed here.

This was marked tracking on web apps initiative, so I do not understand why this is not being tracked on the fennec side. This is additionally breaking marketplace as a packaged app in the process too.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

It is being tracked for web apps, there for does not need to be tracked for fennec

Comment 8

[Wesley Johnston (:wesj)]

Attachment: patch v1

This should disable session restore entirely within webapps. I know this code is a bit fragile, but AFAICT, this has the same behavior (and centralizes this code a bit more).

Comment 9

[Wesley Johnston (:wesj)]

Alternatively, fabrice, can I ping you for two bits of info:

1.) Any idea where B2G does the saved state stuff? I'd like to steal it if we can...

2.) If we don't do that, it would be nice if the app-protocol could walk up the path until it finds a file that does exist (and then pass the path to it so that the page can update its state). Would b2g be ok with that?

Comment 10

[Wesley Johnston (:wesj)]

^

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - What's causing the failure? Can you add some explanation as to why disabling session store fixes this?

Comment 12

[Wesley Johnston (:wesj)]

Do you mean in code? The issue here is that we're loading an app:// protocol at startup, which basically is converted to a jar:// url. If you put a path in that points to a file that isn't inside the jar, we'll fail.

Disabling sessionRestore should mean that we always just load index.html (unless we're already running, in which case we just show whatever is loaded).

Comment 13

[Brian Nicholson (:bnicholson)]

Comment on attachment 737752 [details] [diff] [review]
patch v1

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

Given how frequently Android apps are killed, I hate how our session restore only uses URLs to do restores. It'd be nice if we could serialize the bfcache or something; desktop seems to do a decent job, so perhaps we can pull something from there.

Anyway, for now, a band-aid fix to disable restores sounds fine. An alternative to checking for the existence of a Bundle in onCreate() would be to save the restore state in the outState in onSaveInstanceState(). BrowserApp could set this to RESTORE_OOM, and WebApp could set this to RESTORE_NONE. Then we wouldn't need to create this getSessionRestoreState() method and override it in WebApp.

Comment 14

[[:fabrice] Fabrice Desré]

(In reply to Wesley Johnston (:wesj) from comment #9)
> Alternatively, fabrice, can I ping you for two bits of info:
> 
> 1.) Any idea where B2G does the saved state stuff? I'd like to steal it if
> we can...

We don't save any state at all. It's up to the app to save it state when we close it and restore on startup. We don't even have any session restore in b2g.

> 2.) If we don't do that, it would be nice if the app-protocol could walk up
> the path until it finds a file that does exist (and then pass the path to it
> so that the page can update its state). Would b2g be ok with that?

That's very unlikely. For instance, walking up app://{guid}/foo/bar will never give you app://{guid}/index.html since we don't append the index.html automatically. Also, pushState() being able to change to arbitrary uris, I don't believe that this has too much value to try to do this walking.

Comment 15

[Matt Basta [:basta]]

As a developer, I'd prefer parity between FXOS and FX Mobile. It seems like Android was trying to save/restore state, while FXOS just doesn't ever "shut down" the app, meaning it behaves properly.

The current behavior in Nightly seems to work well, so I'm satisfied with the present state of this issue. Thanks, all

Comment 16

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e88f7f550c2b

Comment 17

[Wesley Johnston (:wesj)]

backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b252af6f343

Comment 18

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/358684072c30

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/358684072c30