jenkins-bot has submitted this change and it was merged.
Change subject: Add DeviceDetectorService class
......................................................................
Add DeviceDetectorService class
In order to remove knowledge about device detection from MobileContext,
including the order of the methods we use to detect devices, it needs to
be moved somewhere: MobileFrontend\Devices\DeviceDetectorService.
If device detection is enabled, then DeviceDetectorService iterates
through a set of child device detectors, the default set of which are
defined in DeviceDetectorService::factory.
Changes:
* Extract DeviceDetectorService from MobileContext#isMobileDevice and
#getDevice.
* Make MobileContext#isMobileDevice use DeviceDetectorService
* Remove MobileContext#getAMF and #getDevice.
Bug: T143891
Change-Id: I09547448f7eb1a56cea718a6751de227d2890987
---
M extension.json
M includes/MobileContext.php
A includes/devices/DeviceDetectorService.php
A tests/phpunit/devices/DeviceDetectorServiceIntegrationTest.php
A tests/phpunit/devices/DeviceDetectorServiceTest.php
5 files changed, 288 insertions(+), 57 deletions(-)
Approvals:
Jdlrobson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 0bd1aad..c4efee7 100644
--- a/extension.json
+++ b/extension.json
@@ -96,7 +96,8 @@
"MobileFrontend\\Devices\\DeviceProperties":
"includes/devices/DeviceProperties.php",
"MobileFrontend\\Devices\\AMFDeviceDetector":
"includes/devices/AMFDeviceDetector.php",
"MobileFrontend\\Devices\\CustomHeaderDeviceDetector":
"includes/devices/CustomHeaderDeviceDetector.php",
- "MobileFrontend\\Devices\\UADeviceDetector":
"includes/devices/UADeviceDetector.php"
+ "MobileFrontend\\Devices\\UADeviceDetector":
"includes/devices/UADeviceDetector.php",
+ "MobileFrontend\\Devices\\DeviceDetectorService":
"includes/devices/DeviceDetectorService.php"
},
"ResourceModules": {
"skins.minerva.base.reset": {
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 2cb8e63..52823d2 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -4,6 +4,7 @@
*/
use MediaWiki\MediaWikiServices;
+use MobileFrontend\Devices\DeviceDetectorService;
/**
* Provide various request-dependant methods to use in mobile context
@@ -53,8 +54,17 @@
*/
protected $analyticsLogItems = [];
- /** @var IDeviceProperties $device Saves current device description */
- private $device;
+ /**
+ * The memoized result of `MobileContext#isMobileDevice`.
+ *
+ * This defaults to `null`, meaning that `MobileContext#isMobileDevice`
has
+ * yet to be called.
+ *
+ * @see MobileContext#isMobileDevice
+ *
+ * @var {bool|null} $isMobileDevice
+ **/
+ private $isMobileDevice = null;
/**
* @var string $action MediaWiki 'action'
@@ -144,31 +154,6 @@
}
/**
- * Gets the current device description
- * @return IDeviceProperties
- */
- public function getDevice() {
- $mobileHeader = $this->getMFConfig()->get( 'MFMobileHeader' );
-
- if ( $this->device ) {
- return $this->device;
- }
- $detector = DeviceDetection::factory();
- $request = $this->getRequest();
-
- if ( $mobileHeader && $this->getRequest()->getHeader(
$mobileHeader ) !== false ) {
- $this->device = new HtmlDeviceProperties();
- } else {
- $userAgent = $request->getHeader( 'User-agent' );
- $acceptHeader = $request->getHeader( 'Accept' );
- $acceptHeader = $acceptHeader === false ? '' :
$acceptHeader;
- $this->device = $detector->detectDeviceProperties(
$userAgent, $acceptHeader );
- }
-
- return $this->device;
- }
-
- /**
* Checks whether references should be lazy loaded for the current user
* @return bool
*/
@@ -214,42 +199,40 @@
}
/**
- * Check whether the device is a mobile device
+ * Detects whether the UA is sending the request from a device and, if
so,
+ * whether to display the mobile view to that device.
+ *
+ * The mobile view will always be displayed to mobile devices. However,
it
+ * will only be displayed to tablet devices if
`$wgMFShowMobileViewToTablets`
+ * is truthy.
+ *
+ * @FIXME: This should be renamed to something more appropriate, e.g.
+ * `shouldDisplayMobileViewToDevice`.
+ *
+ * @see MobileContext::shouldDisplayMobileView
+ *
* @return bool
*/
public function isMobileDevice() {
+ if ( $this->isMobileDevice !== null ) {
+ return $this->isMobileDevice;
+ }
+
+ $this->isMobileDevice = false;
+
$config = $this->getMFConfig();
+ $properties = DeviceDetectorService::factory( $config )
+ ->detectDeviceProperties( $this->getRequest(), $_SERVER
);
- if ( !$config->get( 'MFAutodetectMobileView' ) ) {
- return false;
+ if ( $properties ) {
+ $showMobileViewToTablets = $config->get(
'MFShowMobileViewToTablets' );
+
+ $this->isMobileDevice =
+ $properties->isMobileDevice()
+ || ( $properties->isTabletDevice() &&
$showMobileViewToTablets );
}
- if ( $this->getAMF() ) {
- return true;
- }
- $device = $this->getDevice();
- return $device->isMobileDevice()
- && !( !$config->get( 'MFShowMobileViewToTablets' ) &&
$device->isTablet() );
- }
-
- /**
- * Check for mobile device when using Apache Mobile Filter (AMF)
- *
- * IF AMF is enabled, make sure we use it to detect mobile devices.
- * Tablets are currently served desktop site.
- *
- * AMF docs: http://wiki.apachemobilefilter.org/
- *
- * @return bool
- */
- public function getAMF() {
- $showMobileViewToTablets = $this->getMFConfig()->get(
'MFShowMobileViewToTablets' );
-
- $amf = isset( $_SERVER['AMF_DEVICE_IS_MOBILE'] ) &&
$_SERVER['AMF_DEVICE_IS_MOBILE'] === 'true';
- if ( !$showMobileViewToTablets && $amf ) {
- $amf &= $_SERVER['AMF_DEVICE_IS_TABLET'] === 'false';
- }
- return $amf;
+ return $this->isMobileDevice;
}
/**
diff --git a/includes/devices/DeviceDetectorService.php
b/includes/devices/DeviceDetectorService.php
new file mode 100644
index 0000000..121bdd2
--- /dev/null
+++ b/includes/devices/DeviceDetectorService.php
@@ -0,0 +1,64 @@
+<?php
+
+namespace MobileFrontend\Devices;
+
+use Config;
+use WebRequest;
+
+/**
+ * MobileFrontend's device detector.
+ *
+ * Detects the properties of a device by iterating over a list, //in order//,
+ * child device detectors until one returns a positive result.
+ *
+ * The order of the child device detectors is defined in
+ * `DeviceDetectorService::factory`.
+ */
+class DeviceDetectorService implements DeviceDetector {
+
+ /**
+ * Given MobileFrontend's configuration, creates a new instance of the
device
+ * detector.
+ *
+ * If `$wgMFAutodetectMobileView` is falsy, then no device detection
will
+ * occur.
+ *
+ * @return DeviceDetectorService
+ */
+ public static function factory( Config $config ) {
+ $children = [];
+
+ if ( $config->get( 'MFAutodetectMobileView' ) ) {
+ array_push(
+ $children,
+ new AMFDeviceDetector(),
+ new CustomHeaderDeviceDetector( $config ),
+ new UADeviceDetector()
+ );
+ }
+
+ return new self( $children );
+ }
+
+ /**
+ * @var DeviceDetector[]
+ */
+ private $children;
+
+ /**
+ * @param DeviceDetector[] $children
+ */
+ public function __construct( array $children ) {
+ $this->children = $children;
+ }
+
+ public function detectDeviceProperties( WebRequest $request, array
$server ) {
+ foreach ( $this->children as $child ) {
+ $properties = $child->detectDeviceProperties( $request,
$server );
+
+ if ( $properties ) {
+ return $properties;
+ }
+ }
+ }
+}
diff --git a/tests/phpunit/devices/DeviceDetectorServiceIntegrationTest.php
b/tests/phpunit/devices/DeviceDetectorServiceIntegrationTest.php
new file mode 100644
index 0000000..b297fe6
--- /dev/null
+++ b/tests/phpunit/devices/DeviceDetectorServiceIntegrationTest.php
@@ -0,0 +1,110 @@
+<?php
+
+namespace Tests\MobileFrontend\Devices;
+
+use MediaWikiTestCase;
+use FauxRequest;
+use GlobalVarConfig;
+use MobileFrontend\Devices\DeviceDetectorService;
+
+/**
+ * This suite of test cases tests
+ * `MobileFrontend\Devices\DeviceDetectorService`'s behaviour with no stubbed
+ * dependencies.
+ *
+ * @group integration
+ */
+class DeviceDetectorServiceIntegrationTest extends MediaWikiTestCase {
+ public function setUp() {
+ parent::setUp();
+
+ $this->setMwGlobals( 'wgMFAutodetectMobileView', true );
+
+ $this->request = new FauxRequest();
+ $this->request->setHeader(
+ 'User-Agent',
+
+ // A Macbook Air running Google Chrome (53.0.2785.116).
+ // @codingStandardsIgnoreStart
+ 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6)
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36'
+ // @codingStandardsIgnoreEnd
+ );
+
+ $this->server = [];
+ }
+
+ private function detectDeviceProperties() {
+ $config = new GlobalVarConfig();
+
+ return DeviceDetectorService::factory( $config )
+ ->detectDeviceProperties( $this->request, $this->server
);
+ }
+
+ private function whenTheRequestIsFromAMobileUA() {
+ $this->request->setHeader(
+ 'User-Agent',
+
+ // An iPhone running iOS 8.0.
+ // @codingStandardsIgnoreStart
+ 'Mozilla/6.0 (iPhone; CPU iPhone OS 8_0 like Mac OS X)
AppleWebKit/536.26 (KHTML, like Gecko) Version/8.0 Mobile/10A5376e
Safari/8536.25'
+ // @codingStandardsIgnoreEnd
+ );
+
+ }
+
+ public function test_it_should_handle_requests_from_mobile_UAs() {
+ $this->whenTheRequestIsFromAMobileUA();
+
+ $properties = $this->detectDeviceProperties();
+
+ $this->assertTrue( $properties->isMobileDevice() );
+ $this->assertFalse( $properties->isTabletDevice() );
+ }
+
+ public function test_it_should_handle_a_request_from_desktop_browsers()
{
+ $properties = $this->detectDeviceProperties();
+
+ $this->assertFalse( $properties->isMobileDevice() );
+ $this->assertFalse( $properties->isTabletDevice() );
+ }
+
+ public function test_it_should_prioritize_the_custom_request_header() {
+ // @codingStandardsIgnoreStart
+ // The custom header //should// either be M or ZERO, per
+ //
<https://github.com/wikimedia/operations-puppet/blob/2a2714c28eab25eed469375dc5322ea6a6ef85df/modules/varnish/templates/text-frontend.inc.vcl.erb#L74-L78>.
+ // @codingStandardsIgnoreEnd
+
+ // @FIXME Change the default value of $wgMFMobileHeader to
"X-Subdomain".
+ $this->request->setHeader( 'X-WAP', 'M' );
+
+ $properties = $this->detectDeviceProperties();
+
+ $this->assertTrue( $properties->isMobileDevice() );
+ $this->assertFalse( $properties->isTabletDevice() );
+ }
+
+ /**
+ * @FIXME Should this really be the case?
+ */
+ public function
test_it_should_prioritize_the_amf_environment_variables() {
+ $this->request->setHeader( 'X-WAP', 'M' );
+
+ $this->server[ 'AMF_DEVICE_IS_TABLET' ] = 'true';
+
+ $properties = $this->detectDeviceProperties();
+
+ $this->assertFalse(
+ $properties->isMobileDevice(),
+ 'Apache Mobile Filter environment variables are
prioritized above the custom request header.'
+ );
+ $this->assertTrue( $properties->isTabletDevice() );
+ }
+
+ public function test_it_should_handle_device_detection_being_disabled()
{
+ $this->setMwGlobals( 'wgMFAutodetectMobileView', false );
+
+ $this->whenTheRequestIsFromAMobileUA();
+
+ $this->assertNull( $this->detectDeviceProperties() );
+ }
+}
diff --git a/tests/phpunit/devices/DeviceDetectorServiceTest.php
b/tests/phpunit/devices/DeviceDetectorServiceTest.php
new file mode 100644
index 0000000..3c066a1
--- /dev/null
+++ b/tests/phpunit/devices/DeviceDetectorServiceTest.php
@@ -0,0 +1,73 @@
+<?php
+
+namespace Tests\MobileFrontend\Devices;
+
+use MediaWikiTestCase;
+use MobileFrontend\Devices\DeviceDetector;
+use WebRequest;
+use FauxRequest;
+use MobileFrontend\Devices\DeviceProperties;
+use MobileFrontend\Devices\DeviceDetectorService;
+
+class StubDeviceDetector implements DeviceDetector {
+ public function __construct( $result ) {
+ $this->result = $result;
+ }
+
+ public function detectDeviceProperties( WebRequest $request, array
$server ) {
+ return $this->result;
+ }
+}
+
+class DeviceDetectorServiceTest extends MediaWikiTestCase {
+ protected function setUp() {
+ parent::setUp();
+
+ $this->request = new FauxRequest();
+ }
+
+ /**
+ * Creates a list of child device detectors from a list of results,
which are
+ * then used to create an instance of `CompositeDeviceDetector`.
+ *
+ * @param array $results
+ * @return CompositeDeviceDetector
+ */
+ private function createDetector( array $results ) {
+ $childFactory = function ( $result ) {
+ return new StubDeviceDetector( $result );
+ };
+
+ return new DeviceDetectorService( array_map( $childFactory,
$results ) );
+ }
+
+ public function test_it_should_handle_one_child() {
+ $expectedProperties = new DeviceProperties( true, false );
+ $detector = $this->createDetector( [ $expectedProperties ] );
+
+ $properties = $detector->detectDeviceProperties(
$this->request, [] );
+
+ $this->assertSame( $expectedProperties, $properties );
+ }
+
+ public function test_it_should_handle_many_children() {
+ $expectedProperties = new DeviceProperties( true, false );
+ $detector = $this->createDetector( [
+ null,
+ null,
+ $expectedProperties
+ ] );
+
+ $properties = $detector->detectDeviceProperties(
$this->request, [] );
+
+ $this->assertSame( $expectedProperties, $properties );
+ }
+
+ public function test_it_should_handle_zero_children() {
+ $detector = $this->createDetector( [] );
+
+ $properties = $detector->detectDeviceProperties(
$this->request, [] );
+
+ $this->assertEquals( null, $properties );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/314000
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I09547448f7eb1a56cea718a6751de227d2890987
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Phuedx <samsm...@wikimedia.org>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Phuedx <samsm...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
