Added iOS8 preload-top-hits issue fix for v7 tag #306

Merged
merged 4 commits into from Sep 24, 2015

Projects

None yet

4 participants

@fractaltheory
Member

Status: Ready for Review
Owner: @donnielrt
Reviewers: @haroldtreen @jansepar

Jira Tickets:

Todos:

  • Don +1
  • Shawn +1
  • Minified version

Feedback:

none so far

Changes

  • Add a workaround in the v7 tag for an iOS8 issue, where the autocomplete request for a site sets a mobify-path cookie to blank
  • Use the page visibility API to detect if we're in a background tab, in which case we don't drop an opt-out cookie

How to Test

  • Use Charles to rewrite to this tag, and verify that the mobify-path cookie isn't being set
@donnielrt
Contributor

Thanks for getting this started @fractaltheory, taking a look now.

@donnielrt donnielrt and 1 other commented on an outdated diff Jul 29, 2015
@@ -202,6 +202,21 @@ Private['loadPreview'] = loadPreview;
@type {null}
*/
var disableTag = function() {
+ // This workaround addresses an iOS fallthrough issue, where iOS 8's
+ // "preload top hits" option will load a Mobified page in the
+ // background when typing in an address into the URL bar. Safari
+ // seems to cancel the mobify.js download, resulting in us dropping
+ // a failure cookie.
+ //
+ // We get around this by ignoring mobify.js failures in a background
+ // tab.
+ //
+ // Related ticket: https://mobify.atlassian.net/browse/RTM-280
+ //
+ if (doc.visibilityState && doc.hidden) {
@donnielrt
donnielrt Jul 29, 2015 Contributor

There's no "doc" here :) We need to change this to document

@fractaltheory
fractaltheory Jul 29, 2015 Member

Oh geez. Fixed: 269cb6f

@donnielrt
Contributor

Confirmed that this fixes the issue for the v7 tag as well, woohoo!

👍

@donnielrt
Contributor

@jansepar carrying on the discussion from the v6 tag PR, do you think we need to make this fix opt-out rather than opt-in? I vote yes.

@stewartyu

Is it minified if there are 3 lines? (philosoraptor)

Contributor

Stew, yeah, I'd run it through GCC, not sure if we can get it much smaller. Is that what you meant?

@donnielrt
Contributor

@jansepar can I get a hallelujah?

@jansepar
Member

Looks pretty legit! 👍

@donnielrt
Contributor

Thanks Shawn!

@donnielrt donnielrt merged commit 4a4ff8a into v2.0 Sep 24, 2015

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@donnielrt donnielrt deleted the ios-preload-hit-fix branch Sep 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment