Page MenuHomePhabricator

Differential 1-up render doesn't trigger on iOS
Closed, ResolvedPublic


I have an iPhone 6+s on which I like to punish myself with by reviewing diffs. In general it works out pretty well. However tapping the 'show 20 lines) or 'Show all N lines' links results in a unusable state.

The expanded lines are rendered with a single character per line.


I apologize for the imgur links, but there doesn't seem to be a way to upload files from a mobile device.

Event Timeline

starruler raised the priority of this task from to Needs Triage.
starruler updated the task description. (Show Details)
starruler changed the edit policy from "All Users" to "Custom Policy".
starruler added a subscriber: starruler.

Is SettingsDiff PreferencesShow Unified Diffs set to "On Small Screens" (the default) for you?

Oh, there are only two options and neither would break this.

I suspect we may not be detecting the 6+ screen as "small" enough since it probably has a resolution of 9000x6000 pixels or something, given that it is several times larger than an XBox.

That screenshot was taken as a 'logged out' user. There were two columns (so not unified diffs?)

In Safari/iOS I intermittently get the desktop site. No issues in Chrome.

chad renamed this task from 'Show 20 lines' broken on mobile devices to 'Show 20 lines' broken on mobile Safari.Dec 12 2015, 2:02 AM

image.png (1×750 px, 83 KB)

I was able to repro on Mac OS X with a recent version of chrome.

1/ shrink window to small size
2/ show 20 lines

Screen Shot 2015-12-11 at 6.05.38 PM.png (814×615 px, 84 KB)

The 1-up renderer is server-side, it's not responsive CSS. Reload the page after you shrink.

Ah that makes sense, when I load a page on my iPhone (iOS 9.1) it shows desktop site and flashes down to mobile. I guess the server thinks I'm a desktop. Note this happens even on an iPhone 4s simulator (the oldest version I can run).

Maybe it's an issue with Retina displays and physical vs logical pixels.

See also T10229 for a similar Safari/Diff Render issue.

chad renamed this task from 'Show 20 lines' broken on mobile Safari to Differential 1-up render doesn't trigger on iOS.Jan 28 2016, 5:33 AM

I think D15135 actually fixes this. I can't reproduce it in iOS Simulator after applying that patch.

I've marked D15135 as fixing this out of a general abundance of sunny optimism.

epriestley triaged this task as Normal priority.

Specifically, this is my best guess at what's going on:

  • We correctly guess that the device is probably a phone by examining the User-Agent string and default the page to use the "phone" device class as a starting point, emitting a <body> tag with a "device-phone" CSS class.
  • We emit an appropriate <meta name="viewport" ... /> tag (probably? These tags are a bit black-magic-ish, and maybe the state-of-the-art has changed in the last year or so).
  • Early in the page initialization process, we read the actual viewport dimensions to determine which type of device the client really is. Generally, the intent here is to correct our server-side guess if the UA string wasn't reliable or we guessed wrong.
  • On the affected devices, the viewport width comes back with a number much larger than the device's screen width (for example, iOS Simluator was reading a 900+ pixel device width for an iPhone 4s in portrait mode).
  • We may do bad things (for example, selecting the wrong 1up/2up mode; swapping the page to desktop mode) based on this incorrect reading.
  • Moments later, the reading corrects itself, and we do OK things after that, but we may already have done bad things.

The fix in D15135 is to use the smaller of the measured viewport width or window.screen.availWidth, if the latter is defined. It appears to be defined on affected devices and to present a more reliable way to detect small screen sizes.

This is now deployed to this server ( -- let me know if you're still seeing issues? Behavior seems OK in the simulator and with my physical iPhone 4s, but I don't have a fancy physical 6s to test.