Page MenuHomePhabricator

isWideScreen() should only check the width of the device when determining whether a device is wide screen
Closed, ResolvedPublic3 Estimated Story Points

Description

Problem:
iPhone 6 Plus is considered a tablet.

Expected behavior:
It's a phone.

Root cause:
In resources/mobile.browser/browser.js:

isWideScreen: memoize(function() {
    var val = parseInt(mw.config.get('wgMFDeviceWidthTablet'), 10);
    return window.innerWidth >= val || window.innerHeight >= val;
})

wgMFDeviceWidthTablet is '720px'

The height of iPhone 6 Plus is 736.

Since we check both portrait and landscape mode, the threshold should be the long edge instead of width. Adding one more config variable called wgMFDeviceLongEdgeTablet or wgMFDeviceHeightTablet might make sense. Or just derive from wgMFDeviceWidthTablet * 4/3 (aspect ratio).

According to http://mydevice.io/devices/, 800px looks like a good threshold for the long edge measure.

This issue is discovered in https://bugs.chromium.org/p/chromium/issues/detail?id=647667#c14 because section collapsing is determined by whether it's a phone or tablet.

Acceptance criteria

  • From now on we should only check the width of the device.
isWideScreen: memoize(function() {
    var val = parseInt(mw.config.get('wgMFDeviceWidthTablet'), 10);
    return window.innerWidth >= val;
})
  • Add some documentation explaining that we do not check the height to avoid situations where tablet and mobile modes differ.
  • Ensure any JS specific media queries match the definition of "widescreen" ie. if isWideScreen is false AND media rules apply the UI should continue to be functional.

QA steps

Use an iPhone XS in browser stack in portrait mode.
Load an article e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Qatar
Verify that the sections are collapsed by default

Switch to landscape
Refresh the page
Verify the sections are expanded by default

QA Results

ACStatusDetails
1T159475#5083960
2T159475#5083960

Event Timeline

Wouldn't it be better to keep the value of wgMFDeviceWidthTablet as is and always compare it to window.innerWidth? This way iPhone 6 Plus won't be considered a tablet when it is in a portrait mode, but will be when it is in landscape mode.

I don't really use phones in landscape mode for web browsing, but from the comment, it seems it's desirable to keep the behavior the same for both portrait and landscape modes. The section folding is only done when the document is loaded, but not on device rotation. If we load the page in landscape mode, the section wouldn't be folded, and when changing to portrait, it would still not be folded, unlike if we load the document in portrait in the beginning.

What @bmansurov says is correct. It should behave as tablet in landscape and mobile in portrait.

So let's use width. It's simpler.

As for the threshold, should it be something closer to 600 instead of 720? It's a better cut according to http://mydevice.io/devices/.

That would be a question for @Nirzar and we should track that in a separate bug since when we change the threshold we have to be careful not to break layouts.

@Nirzar seems like an easy fix...can you tell us how it should behave?

Jdlrobson triaged this task as Medium priority.Nov 13 2018, 9:50 PM
Jdlrobson moved this task from Needs analysis to Queue on the Web-Team-Backlog (Design) board.
Jdlrobson added a subscriber: alexhollender_WMF.

@alexhollender this should be an easy fix, once I have what's hopefully a quick answer from you.

@alexhollender
The open question is whether switching to landscape mode should trigger the tablet mode on devices such as iPhone 6 Plus.
Currently iPhone 6 Plus behaves like a mobile device in both portrait and landscape mode.
The request is to make it behave like a mobile device in portrait mode, but a tablet device in landscape mode.

@Jdlrobson maybe some additional clarification is needed — you said:

The request is to make it behave like a mobile device in portrait mode, but a tablet device in landscape mode.

from T159475#3076651

...it seems it's desirable to keep the behavior the same for both portrait and landscape modes

I agree with @wychen that based on how collapsable sections behave, wide but short screens (i.e. phones in landscape orientation) should render the mobile version of the site.

  • are there any other drawbacks to loading the tablet version on wide but short screens? I think it'd be valuable to have them documented
  • I imagine there will always be phones that, when in landscape orientation, will be as wide or wider than some tablets. Do we then need to take into consideration width and height of the screen?
Jdlrobson renamed this task from isWideScreen() should use the long edge instead of wgMFDeviceWidthTablet as the threshold to isWideScreen() should only check the width of the device when determining whether a device is wide screen.Dec 11 2018, 7:55 PM
Jdlrobson edited projects, added Web-Team-Backlog; removed Web-Team-Backlog (Design).
Jdlrobson updated the task description. (Show Details)
Jdlrobson raised the priority of this task from Medium to High.Mar 13 2019, 5:53 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.
Jdlrobson added a subscriber: Edtadros.

This tripped up @Edtadros today when testing on an iPhone X! This is causing unnecessary confusion during development and QA and I'll liked us to fix this as soon as we can/

ovasileva set the point value for this task to 3.Mar 27 2019, 4:45 PM

Change 501050 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Remove window.innerHeight check from Browser.isWideScreen

https://gerrit.wikimedia.org/r/501050

Change 501050 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove window.innerHeight check from Browser.isWideScreen

https://gerrit.wikimedia.org/r/501050

Test Result:

Status: ✅ PASS
OS: iOS
Browser: Safari
Device: iPhone X

Test Artifact(s):

QA steps
Use an iPhone XS in browser stack in portrait mode.
Test was executed on an iPhone X with the same resolution as an iPhone XS

✅ AC1: Load an article e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Qatar
Verify that the sections are collapsed by default

IMG_3679.jpeg (2×1 px, 212 KB)

✅ AC2: Switch to landscape
Refresh the page
Verify the sections are expanded by default

IMG_3680.jpeg (1×2 px, 352 KB)

IMG_3681.jpeg (1×2 px, 347 KB)