codeforamerica / MuniciPal

:speech_balloon: Consulting city-dwellers about legislation near them.
22 stars 14 forks source link

App continuously reloads itself on iOS (Safari & Chrome) #130

Closed vancefsmithmesa closed 9 years ago

vancefsmithmesa commented 9 years ago

When the application is viewed on any iOS device (phone, ipad) on Safari or Chrome the app continuously reloads itself every couple seconds. As you would imagine, it makes the application unusable on these devices. Curiously, the app works fine from a Windows desktop or from an Android phone.

We see this behavior in the staging site you set up for us on 11/12 (http://mesamunicipal-staging.herokuapp.com) as well as a staging app that we created today (11/24) for testing purposes from the CfA/MuniciPal master (http://mesamunistaging.herokuapp.com ).

ardouglass commented 9 years ago

This is being caused by the back button fix from 3b5ce15e2649da31b959dadc7c8237fcf6a79fed.

If you look at MDN's documentation on popstate events, they mention that Safari and older versions of Chrome (I'm guessing webkit) fire a popstate event on page load.

In addresses_onload.js, an event listener gets set up that listens for popstate events, and then calls loadState(event.state). The event doesn't seem to contain any data, so when loadState() gets called and nothing gets passed to it, it does the else portion and just reloads the page with window.location.reload(); over and over.

@techieshark what is the purpose of the window.location.reload(); in that part of the code?

techieshark commented 9 years ago

Ah, interesting. Thanks for reporting, @vancefsmithmesa and thanks for digging that up, @ardouglass.

@ardouglass the window reload is because when you click back a bunch of times (after clicking around the site a number of times), you'll be popping window states off a stack until you get to the bottom of the stack (which I hoped meant that you had clicked back enough so that you had reached the first page you ever came to, and in that instance we wanted to reset the page to it's initial state and the easiest way to do that was via a page reload).

@ardouglass perhaps we could push some kind of initial state on page load to differentiate between Safari / pre v34 Chrome popstate on page load and a back button click induced popstate. What do you think?

ardouglass commented 9 years ago

After more digging, it looks like the problem is with the mixing of navigation types. When a user clicks a link the browser creates an item in history, but there isn't a state object attached, meaning history items created by navigation will force a page refresh through loadState() where the items created by dragging the pointer in the map will have a state attached and reload without forcing a page refresh.

@techieshark I don't know that adding an initial state on page load would help by itself since the thing making non-pushed history items load forces a page refresh and nothing in the javascript would persist. It would probably involve cookies that also get updated alongside state data. I also tried to base it on window.history.length but this doesn't only account for pages at your domain and will keep adding up across the full history of the tab.

The quick solution seems to be to remove the pin navigation, which if I recall was one of the more confusing features for users anyway. It could also not update the history of the back button, though that's probably confusing for anyone who does use it.

The long solution involves making every link on the page work as a pushState. Also, supposedly pushState doesn't work below IE10, so that's another relevant thing to consider.

techieshark commented 9 years ago

Interesting, @ardouglass. Would you be able to try the branch I just pushed (https://github.com/codeforamerica/MuniciPal/tree/fixes/130-continuous-reloading) in chrome, safari, and IE and let me know what you find?

vancefsmithmesa commented 9 years ago

Will do.

Joyce/Eric: can you test this?

Vance F Smith | IT Engineer III | Information Technology | O: 480-644-3243

From: Peter W [mailto:notifications@github.com] Sent: Tuesday, November 25, 2014 1:12 PM To: codeforamerica/MuniciPal Cc: Vance Smith Subject: Re: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

Interesting, @ardouglasshttps://github.com/ardouglass. Would you be able to try the branch I just pushed (https://github.com/codeforamerica/MuniciPal/tree/fixes/130-continuous-reloading) in chrome, safari, and IE and let me know what you find?

— Reply to this email directly or view it on GitHubhttps://github.com/codeforamerica/MuniciPal/issues/130#issuecomment-64463170.

vancefsmithmesa commented 9 years ago

Did the code get pushed out there yet or do we need to merge it to our branch?

I just tested using the last url we had from cfa http://mesamunicipal-staging.herokuapp.com/ and found the same result. Is there another url to use?

From: Vance Smith Sent: Wednesday, November 26, 2014 7:48 AM To: codeforamerica/MuniciPal; codeforamerica/MuniciPal Cc: Joyce Turner; Eric Thompson Subject: RE: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

Will do.

Joyce/Eric: can you test this?

Vance F Smith | IT Engineer III | Information Technology | O: 480-644-3243

From: Peter W [mailto:notifications@github.com] Sent: Tuesday, November 25, 2014 1:12 PM To: codeforamerica/MuniciPal Cc: Vance Smith Subject: Re: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

Interesting, @ardouglasshttps://github.com/ardouglass. Would you be able to try the branch I just pushed (https://github.com/codeforamerica/MuniciPal/tree/fixes/130-continuous-reloading) in chrome, safari, and IE and let me know what you find?

— Reply to this email directly or view it on GitHubhttps://github.com/codeforamerica/MuniciPal/issues/130#issuecomment-64463170.

techieshark commented 9 years ago

@vancefsmithmesa, can you try again on the staging site? I tried pushing it there but am not sure if it worked (I have very limited 'net connection today). You can also test the branch directly, I think, by deploying that branch to heroku using the button (you'd want to create a -testing app for yourself for that).

ardouglass commented 9 years ago

@techieshark I tested it in Safari on my Mac, an iPhone, and an iPad. Everything worked well. It also didn't break navigation when moving the map pin like the fix I was toying with. =)

techieshark commented 9 years ago

:+1: thanks @ardouglass!

vancefsmithmesa commented 9 years ago

Hi, sorry for the delay – I was out most of last week. I will test right now.

Vance F Smith | IT Engineer III | Information Technology | O: 480-644-3243

From: Peter W [mailto:notifications@github.com] Sent: Wednesday, November 26, 2014 10:15 AM To: codeforamerica/MuniciPal Cc: Vance Smith Subject: Re: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

@vancefsmithmesahttps://github.com/vancefsmithmesa, can you try again on the staging site? I tried pushing it there but am not sure if it worked (I have very limited 'net connection today). You can also test the branch directly, I think, by deploying that branch to heroku using the button (you'd want to create a -testing app for yourself for that).

— Reply to this email directly or view it on GitHubhttps://github.com/codeforamerica/MuniciPal/issues/130#issuecomment-64679387.

vancefsmithmesa commented 9 years ago

I tested in staging on my iphone and ipad and the app seems to be working well. Thanks @techiesharkhttps://github.com/techieshark and @ardouglasshttps://github.com/ardouglass for your quick assistance on this.

Vance F Smith | IT Engineer III | Information Technology | O: 480-644-3243

From: Vance Smith Sent: Monday, December 01, 2014 9:38 AM To: 'codeforamerica/MuniciPal'; codeforamerica/MuniciPal Cc: Joyce Turner; Eric Thompson Subject: RE: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

Hi, sorry for the delay – I was out most of last week. I will test right now.

Vance F Smith | IT Engineer III | Information Technology | O: 480-644-3243

From: Peter W [mailto:notifications@github.com] Sent: Wednesday, November 26, 2014 10:15 AM To: codeforamerica/MuniciPal Cc: Vance Smith Subject: Re: [MuniciPal] App continuously reloads itself on iOS (Safari & Chrome) (#130)

@vancefsmithmesahttps://github.com/vancefsmithmesa, can you try again on the staging site? I tried pushing it there but am not sure if it worked (I have very limited 'net connection today). You can also test the branch directly, I think, by deploying that branch to heroku using the button (you'd want to create a -testing app for yourself for that).

— Reply to this email directly or view it on GitHubhttps://github.com/codeforamerica/MuniciPal/issues/130#issuecomment-64679387.

techieshark commented 9 years ago

Thanks @vancefsmithmesa and @ardouglass!