Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iPad2 iOS7 Offline use not clearing images in background #174

Closed
gruneich opened this issue Oct 28, 2013 · 17 comments
Closed

iPad2 iOS7 Offline use not clearing images in background #174

gruneich opened this issue Oct 28, 2013 · 17 comments

Comments

@gruneich
Copy link

I'll fill out this issue fully when Ariel and Oliver get back from the conference as they took the iPad2.

@ghost ghost assigned jonathanolson Oct 29, 2013
@samreid
Copy link
Member

samreid commented Oct 29, 2013

I also reproduced this problem on iPad3/iOS7 for a single file build of John Travoltage. But the ipad has to be in airplane mode (or wireless off) for the bug to manifest.

@pixelzoom
Copy link
Contributor

On 10/29/13 Skype developer call...

We reproduced this on iPad2 and iPad3, with iOS6 and iOS7, using a couple of different sims (john-travoltage, molarity, build-an-atom). Steps to reproduce:

  1. Open a sim in Safari
  2. Add the sim to reading list
  3. Turn off WiFi connection in Settings (if you skip this step, things will appear to work ok)
  4. Select the sim from reading list
  5. The sim will be visually messed up. (Eg, John Travoltage will have too many arms and legs. BAA has homescreen text that isn't cleared. Etc.)

@jbphet checked for console output and there is none.

@jonathanolson will take a look at this.

Whether to address this is a question we should put to @arielphet.

@jbphet
Copy link
Contributor

jbphet commented Oct 29, 2013

Email sent to @ariel-phet and @kathy-phet:

AP and KP,

Off line support of simulations (a.k.a. ‘Reading List’) appears to have some issues on iPad (see #174). During today’s developer meeting, we duplicated the issues on several different iPads and both iOS6 and iOS7. I tethered my iPad to my Mac to see if there was any console output that might lend some insight, but there was not. Our current impression is that this will not be a trivial issue to solve. In our opinion, the ‘Reading List’ is a fairly advanced feature, and it may not be used by many schools, but we don’t really know. We do know that [other PhET team members] wanted support for this in order to demo the sims at conferences. All in all, it is hard for we developers to gauge the importance and priority of making this work. Can we get some guidance from you on it? It would likely be @jonathanolson that would have to do at least the initial investigations as to why it currently doesn’t work.

You can enter your thoughts in the ticket if you like.

@jbphet
Copy link
Contributor

jbphet commented Oct 29, 2013

Screen capture of this issue as manifested for the Build an Atom simulation. Note the presence of the 'Atom' caption, the 'Symbol' caption, and part of the 'Build an Atom' caption from the home screen:

baa-off-line-ipad-shows-issues

This was captured on an iPad 2, iOS 6.0.1.

@pixelzoom
Copy link
Contributor

Screenshots of john-travoltage 1.0.0-rc.1, in landscape and portrait orientations. Extra arms & legs in both orientations, severed limbs in portrait. Also extra navbar in portrait.

travoltage-landscape

travoltage-portrait

@jbphet
Copy link
Contributor

jbphet commented Oct 30, 2013

@kathy-phet said (in a email):

Hi Folks,

It's too bad that the 'Reading List' doesn't work exactly as hoped.

Given everything else on our plates - I would think that this feature could be put on the back-burner for a while with the intent to revisit it later down the road?
@ariel-phet - do you agree?

@jonathanolson
Copy link
Contributor

We should check the iOS 7.1 behavior to see if this was fixed.

@jbphet
Copy link
Contributor

jbphet commented Apr 10, 2014

Discussed with @ariel-phet 4/10/2014 developer meeting, and he suggested that we work on this after we finish the round of redeployments that are currently planned. Best guess right now is that it will be around the end of May 2014.

@pixelzoom
Copy link
Contributor

@jonathanolson reported: You can tether to a sim in the reading list. There's nothing reported in the console. There are 2 scene divs appearing in the dom, which is probably the source of the problem.

@jonathanolson
Copy link
Contributor

Will test to see if Joist gets initializes twice.

@samreid
Copy link
Member

samreid commented Apr 10, 2014

Printing "console.log('new sim');" from Sim.js only shows up once. However, If I call

    console.log( 'before', $( 'body #sim' ) );

before the sim div is added to the body (in Sim.js), it shows that there was already a sim div in the body, so the one created in Sim.js is a duplicate. I tried this workaround and it seemed to fix the problem:

    if ( $( 'body #sim' ).length > 0 ) {
      $body.empty();
    }
    $body.append( $simDiv );
    this.$simDiv = $simDiv;

However, I am not sure if there are other elements in the body that must be kept.

@samreid
Copy link
Member

samreid commented Apr 10, 2014

One hypothesis consistent with the above observed behavior is that Safari creates a snapshot of the DOM when something is stored in the reading list. Then when the item is viewed in the reading list, it starts with the cached DOM then runs all the JS again. In our case, running the JS again creates another "live" copy of the DOM.

@jonathanolson
Copy link
Contributor

Joist doesn't place the localized strings and locale specifier, so removing those from the body could potentially create issues with i18n. There's also the off-chance that any scripts added in the body after our main require.js-minified code wouldn't get executed if we empty the body (I forget the exact handling for script execution).

Otherwise, I agree that it seems like it takes a DOM snapshot (maybe we can add a setting to prevent this), and removing the dynamically-added content (Scenery scene, Scenery "stub Text size tester", Google Analytics script, audio elements, etc.) would be good.

It's likely that "offline reading" mode is causing two Google Analytics pageviews to be sent right now, and we'd want to make sure we only send one (or zero, or a offline-tagged pageview) when it is loaded in "offline reading".

jonathanolson added a commit to phetsims/joist that referenced this issue Apr 11, 2014
@jonathanolson
Copy link
Contributor

Google Analytics only gets run once, since the existence of the script in the DOM doesn't actually change anything for the reading list.

Otherwise, it should be fixed up now. Will wait for someone else to verify.

@samreid
Copy link
Member

samreid commented Apr 11, 2014

Looks fixed on my iPhone 5S (I didn't attempt to verify that the google analytics only runs once, but everything else seems great).

@ycarpenterphet provided a nice bug report for this on acid base solutions (see issue number above), perhaps it would be good for her to verify the fix too.

@samreid
Copy link
Member

samreid commented Apr 11, 2014

I confirmed this is fixed on my iPad3/iOS7. I'll send an email to @ycarpenterphet that she can test the next acid-base-solutions if she wants. Closing.

@samreid samreid closed this as completed Apr 11, 2014
@samreid
Copy link
Member

samreid commented Apr 11, 2014

By the way, nice work @jonathanolson !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants