nit: Minor code cleanup

- Only add privacy url to evl when consent is required - Change flow in videolink.js - Catch possible json exception in getIframeConfig
StarCitizenWiki · Oct 17, 2023 · 979bacf · 979bacf
1 parent c42adcf
commit 979bacf
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 35 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -0,0 +1,6 @@
+/.storybook/
+/docs/
+/i18n/
+/node_modules/
+/vendor/
+Gruntfile.js
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -0,0 +1,20 @@
+{
+	"root": true,
+	"extends": [
+		"wikimedia/client",
+		"wikimedia/jquery",
+		"wikimedia/mediawiki"
+	],
+	"globals": {
+		"require": "readonly",
+		"module": "readonly"
+	},
+	"rules": {
+		"one-var": "off",
+		"//": [
+			"off",
+			"ResourceLoader's `packageFiles` do not require wrapping but the `module` option is only available in ES6+."
+		],
+		"no-implicit-globals": "off"
+	}
+}
diff --git a/.gitignore b/.gitignore
@@ -4,4 +4,7 @@
 .DS_Store
 .svn
 /vendor
-composer.lock
+/composer.lock
+/node_modules
+/.eslintcache
+/package-lock.json
diff --git a/includes/EmbedService/AbstractEmbedService.php b/includes/EmbedService/AbstractEmbedService.php
@@ -485,9 +485,8 @@ public function getTitle(): ?string {
 	 * @param null|int $width
 	 * @param null|int $height
 	 * @return string
-	 * @throws JsonException
 	 */
-	public function getIframeConfig( ?int $width = 0, ?int $height = 0 ): string {
+	public function getIframeConfig( $width = 0, $height = 0 ): string {
 		$attributes = [];
 		if ( !empty( $width ) && $width !== $this->getDefaultWidth() ) {
 			$attributes['width'] = $width;
@@ -498,7 +497,11 @@ public function getIframeConfig( ?int $width = 0, ?int $height = 0 ): string {
 
 		$attributes['src'] = $this->getUrl();
 
-		return json_encode( $attributes, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES );
+		try {
+			return json_encode( $attributes, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES );
+		} catch ( JsonException $e ) {
+			return '{"error": "Could not encode iframe config"}';
+		}
 	}
 
 	/**

diff --git a/includes/EmbedVideo.php b/includes/EmbedVideo.php
@@ -185,12 +185,15 @@ public static function parseEVL( Parser $parser, PPFrame $frame, array $args ):
 		$linkConfig = [
 			'data-iframeconfig' => $ev->service->getIframeConfig( $ev->args['width'], $ev->args['height'] ),
 			'data-service' => $ev->args['service'],
-			'data-privacy-url' => $ev->service->getPrivacyPolicyUrl(),
 			'data-player' => $ev->args['player'] ?? 'default',
 			'class' => 'embedvideo-evl vplink',
 			'href' => '#',
 		];
 
+		if ( MediaWikiServices::getInstance()->getMainConfig()->get( 'EmbedVideoRequireConsent' ) === true ) {
+			$linkConfig['data-privacy-url'] = $ev->service->getPrivacyPolicyUrl();
+		}
+
 		return [
 			Html::element( 'a', $linkConfig, $ev->args['text'] ),
 			'noparse' => true,
@@ -366,7 +369,7 @@ private function parseArgs( array $args, bool $fromTag ): array {
 			if ( ( $keys[$counter] !== 'urlArgs' || str_contains( $arg, 'urlArgs' ) ) && preg_match( '/https?:/', $arg ) !== 1 ) {
 				$pair = explode( '=', $arg, 2 );
 			}
-			$pair = array_map( 'trim', $pair );
+			$pair = array_map( 'strip_tags', array_map( 'trim', $pair ) );
 
 			// We are handling a named argument
 			if ( count( $pair ) === 2 ) {

diff --git a/package.json b/package.json
@@ -0,0 +1,16 @@
+{
+	"private": true,
+	"scripts": {
+		"lint:fix:js": "npm -s run lint:js -- --fix",
+		"lint:js": "eslint --cache --max-warnings 0 ."
+	},
+	"devDependencies": {
+		"grunt": "1.3.0",
+		"grunt-banana-checker": "0.9.0",
+		"eslint-config-wikimedia": "0.17.0",
+		"jsdoc": "3.6.3",
+		"jsdoc-wmf-theme": "0.0.3",
+		"stylelint-config-idiomatic-order": "8.1.0",
+		"stylelint-config-wikimedia": "0.10.3"
+	}
+}
diff --git a/resources/ext.embedVideo.consent.js b/resources/ext.embedVideo.consent.js
@@ -1,4 +1,4 @@
-const makeIframe = require('./iframe.js').makeIframe;
+const { makeIframe } = require('./iframe.js');
 
 (function () {
 	mw.hook( 'wikipage.content' ).add( () => {

diff --git a/resources/ext.embedVideo.videolink.js b/resources/ext.embedVideo.videolink.js
@@ -5,54 +5,57 @@ const {makeIframe, fetchThumb} = require('./iframe.js');
         document.querySelectorAll('.embedvideo-evl').forEach(function (evl) {
             evl.addEventListener('click', e => {
                 e.preventDefault();
-                e.stopPropagation();
 
                 const player = evl?.dataset?.player ?? 'default';
                 const iframeConfig = JSON.parse(evl.dataset.iframeconfig);
 
                 const iframe = document.querySelector(`.embedvideo.evlplayer-${player} iframe`);
+                // Iframe exists, no consent required or already given
                 if (iframe !== null) {
                     for (const [key, value] of Object.entries(iframeConfig)) {
                         iframe.setAttribute(key, value);
                     }
-                } else {
-                    const div = document.querySelector(`.embedvideo.evlplayer-${player}`);
 
-                    if (div === null) {
-                        console.warn(`No player with id '${player}' found!.`);
-                        return;
-                    }
+                    return;
+                }
+
+                // No iframe exists, only when explicit consent is required
+                const div = document.querySelector(`.embedvideo.evlplayer-${player}`);
+
+                if (div === null || evl.dataset?.iframeconfig === null) {
+                    console.warn(`No player with id '${player}' found!.`);
+                    return;
+                }
 
-                    const wrapper = div.querySelector('.embedvideo-wrapper');
-                    const consentDiv = wrapper.querySelector('.embedvideo-consent');
+                const wrapper = div.querySelector('.embedvideo-wrapper');
+                const consentDiv = wrapper.querySelector('.embedvideo-consent');
 
-                    const origService = div?.dataset?.service;
+                const origService = div.dataset?.service;
 
-                    div.dataset.iframeconfig = evl.dataset.iframeconfig;
-                    div.dataset.service = evl.dataset.service;
+                div.dataset.iframeconfig = evl.dataset.iframeconfig;
+                div.dataset.service = evl.dataset.service;
 
-                    const message = 'embedvideo-service-' + evl.dataset.service;
-                    const service = mw.message(message).escaped();
+                const serviceMessage = mw.message('embedvideo-service-' + (evl.dataset?.service ?? 'youtube')).escaped();
+                const privacyMessage = mw.message('embedvideo-consent-privacy-notice-text', serviceMessage).escaped();
 
+                div.querySelector('.embedvideo-loader__service').innerText = serviceMessage;
+                div.querySelector('.embedvideo-privacyNotice__content').innerText = privacyMessage;
+
+                if (evl.dataset?.privacyUrl !== null) {
                     const link = document.createElement('a');
-                    link.href = evl?.dataset?.privacyUrl ?? '#';
+                    link.href = evl.dataset.privacyUrl;
                     link.rel = 'nofollow,noopener';
                     link.target = '_blank';
                     link.classList.add('embedvideo-privacyNotice__link');
                     link.innerText = mw.message('embedvideo-consent-privacy-policy').escaped();
 
-                    div.querySelector('.embedvideo-loader__service').innerText = service;
-                    div.querySelector('.embedvideo-privacyNotice__content').innerText = mw.message('embedvideo-consent-privacy-notice-text', service).escaped();
-
-                    if (link.href !== '#') {
-                        div.querySelector('.embedvideo-privacyNotice__content').appendChild(link);
-                    }
+                    div.querySelector('.embedvideo-privacyNotice__content').appendChild(link);
+                }
 
-                    if (origService === 'videolink') {
-                        makeIframe(div);
-                    } else {
-                        fetchThumb(iframeConfig.src, consentDiv, wrapper.parentElement);
-                    }
+                if (origService === 'videolink') {
+                    makeIframe(div);
+                } else {
+                    fetchThumb(iframeConfig.src, consentDiv, wrapper.parentElement);
                 }
             });
         });

diff --git a/resources/modules/iframe.js b/resources/modules/iframe.js
@@ -1,4 +1,3 @@
-
 const fetchThumb = async (url, parent, container) => {
     const fetcherFactory = require('./fetchFactory.js').fetchFactory;
 
@@ -119,7 +118,6 @@ const makeIframe = function(ev) {
         let iframeConfig = ev.dataset.iframeconfig;
 
         iframeConfig = {
-            ...mw.config.get('ev-default-config') ?? [],
             ...mw.config.get('ev-' + ev.dataset.service + '-config') ?? [],
             ...JSON.parse(iframeConfig)
         };