From aaa5ad254be12aa69f8e6efdf88faafc0e8cbedc Mon Sep 17 00:00:00 2001 From: Ed Sanders Date: Sun, 26 May 2013 16:23:03 +0100 Subject: [PATCH] Implement new browser compatibility checks We now have three stages: 1. Browser feature tests. Dies silently if any fail. 2. Browser blacklist. Dies silently if match found. 3. Browser whitelist. Shows warning if no match found. Previously we were treating the remotely generated edit notices as if they were in an object when in fact they were in an array - the code has been fixed to reflect that fact. As locally generated notices will typically require parsed messages, we've also moved the notice rendering to after onReady is fired. Updated jquery.client to latest master from MediaWiki core (needed for proper detection of Iceweasel, Android and Safari) Bug: 38128 Change-Id: Idc5f4a23a2709264d869a91d00873c4e187bc470 --- VisualEditor.i18n.php | 1 + VisualEditorMessagesModule.php | 1 + modules/jquery/jquery.client.js | 67 ++++++++------ .../mw/targets/ve.init.mw.ViewPageTarget.js | 64 ++++++++------ modules/ve/init/mw/ve.init.mw.Target.js | 87 ++++++++++++------- 5 files changed, 136 insertions(+), 84 deletions(-) diff --git a/VisualEditor.i18n.php b/VisualEditor.i18n.php index d18f410f3e..0225f90ea2 100644 --- a/VisualEditor.i18n.php +++ b/VisualEditor.i18n.php @@ -101,6 +101,7 @@ $messages['en'] = array( 'visualeditor-aliennode-tooltip' => 'Sorry, this element cannot be edited using the VisualEditor', 'visualeditor-descriptionpagelink' => 'Project:VisualEditor', 'visualeditor-alphawarning' => 'You are using an alpha version of the [[{{MediaWiki:Visualeditor-descriptionpagelink}}|VisualEditor]]. It may be slow and make erroneous changes—please check each edit that you make.', + 'visualeditor-browserwarning' => 'You are using a browser which is not officially supported by VisualEditor.', 'visualeditor-report-notice' => 'I understand that by clicking "Report problem" I will transmit my changes and my feedback, which will be stored for analysis. I agree to provide feedback in accordance with the [[{{MediaWiki:Visualeditor-report-link}}|Terms of Use]].', 'visualeditor-report-link' => 'foundation:Terms of Use', 'visualeditor-feedback-link' => 'Project:VisualEditor/Feedback', diff --git a/VisualEditorMessagesModule.php b/VisualEditorMessagesModule.php index 6821be6ca7..49eb094d21 100644 --- a/VisualEditorMessagesModule.php +++ b/VisualEditorMessagesModule.php @@ -44,6 +44,7 @@ class VisualEditorMessagesModule extends ResourceLoaderModule { $msgArgs = array( 'minoredit' => array( 'minoredit' ), 'watchthis' => array( 'watchthis' ), + 'visualeditor-browserwarning' => array( 'visualeditor-browserwarning' ), 'visualeditor-report-notice' => array( 'visualeditor-report-notice' ), 'missingsummary' => array( 'missingsummary' ), ); diff --git a/modules/jquery/jquery.client.js b/modules/jquery/jquery.client.js index b0bd6850b6..2da022c554 100644 --- a/modules/jquery/jquery.client.js +++ b/modules/jquery/jquery.client.js @@ -6,7 +6,7 @@ /* Private Members */ /** - * @var profileCache {Object} Keyed by userAgent string, + * @var {Object} profileCache Keyed by userAgent string, * value is the parsed $.client.profile object for that user agent. */ var profileCache = {}; @@ -18,9 +18,9 @@ /** * Get an object containing information about the client. * - * @param nav {Object} An object with atleast a 'userAgent' and 'platform' key. + * @param {Object} nav An object with atleast a 'userAgent' and 'platform' key. * Defaults to the global Navigator object. - * @return {Object} The resulting client object will be in the following format: + * @returns {Object} The resulting client object will be in the following format: * { * 'name': 'firefox', * 'layout': 'gecko', @@ -50,11 +50,11 @@ // Generic version digit x = 'x', // Strings found in user agent strings that need to be conformed - wildUserAgents = ['Opera', 'Navigator', 'Minefield', 'KHTML', 'Chrome', 'PLAYSTATION 3'], + wildUserAgents = ['Opera', 'Navigator', 'Minefield', 'KHTML', 'Chrome', 'PLAYSTATION 3', 'Iceweasel'], // Translations for conforming user agent strings userAgentTranslations = [ // Tons of browsers lie about being something they are not - [/(Firefox|MSIE|KHTML,\slike\sGecko|Konqueror)/, ''], + [/(Firefox|MSIE|KHTML,?\slike\sGecko|Konqueror)/, ''], // Chrome lives in the shadow of Safari still ['Chrome Safari', 'Chrome'], // KHTML is the layout engine not the browser - LIES! @@ -70,14 +70,14 @@ // version detectection versionPrefixes = [ 'camino', 'chrome', 'firefox', 'iceweasel', 'netscape', 'netscape6', 'opera', 'version', 'konqueror', - 'lynx', 'msie', 'safari', 'ps3' + 'lynx', 'msie', 'safari', 'ps3', 'android' ], // Used as matches 2, 3 and 4 in version extraction - 3 is used as actual version number versionSuffix = '(\\/|\\;?\\s|)([a-z0-9\\.\\+]*?)(\\;|dev|rel|\\)|\\s|$)', // Names of known browsers names = [ 'camino', 'chrome', 'firefox', 'iceweasel', 'netscape', 'konqueror', 'lynx', 'msie', 'opera', - 'safari', 'ipod', 'iphone', 'blackberry', 'ps3', 'rekonq' + 'safari', 'ipod', 'iphone', 'blackberry', 'ps3', 'rekonq', 'android' ], // Tanslations for conforming browser names nameTranslations = [], @@ -173,46 +173,61 @@ }, /** - * Checks the current browser against a support map object to determine if the browser has been black-listed or - * not. If the browser was not configured specifically it is assumed to work. It is assumed that the body - * element is classified as either "ltr" or "rtl". If neither is set, "ltr" is assumed. + * Checks the current browser against a support map object. * * A browser map is in the following format: * { + * // Multiple rules with configurable operators + * 'msie': [['>=', 7], ['!=', 9]], + * // Match no versions + * 'iphone': false, + * // Match any version + * 'android': null + * } + * + * It can optionally be split into ltr/rtl sections: + * { * 'ltr': { - * // Multiple rules with configurable operators - * 'msie': [['>=', 7], ['!=', 9]], - * // Blocked entirely + * 'android': null, * 'iphone': false * }, * 'rtl': { - * // Test against a string - * 'msie': [['!==', '8.1.2.3']], - * // RTL rules do not fall through to LTR rules, you must explicity set each of them + * 'android': false, + * // rules are not inherited from ltr * 'iphone': false * } * } * - * @param map {Object} Browser support map - * @param profile {Object} (optional) a client-profile object. + * @param {Object} map Browser support map + * @param {Object} [profile] A client-profile object + * @param {boolean} [exactMatchOnly=false] Only return true if the browser is matched, otherwise + * returns true if the browser is not found. * - * @return Boolean true if browser known or assumed to be supported, false if blacklisted + * @returns {boolean} The current browser is in the support map */ - test: function ( map, profile ) { + test: function ( map, profile, exactMatchOnly ) { /*jshint evil: true */ var conditions, dir, i, op, val; profile = $.isPlainObject( profile ) ? profile : $.client.profile(); - dir = $( 'body' ).is( '.rtl' ) ? 'rtl' : 'ltr'; - // Check over each browser condition to determine if we are running in a compatible client - if ( typeof map[dir] !== 'object' || map[dir][profile.name] === undefined ) { - // Unknown, so we assume it's working - return true; + if ( map.ltr && map.rtl ) { + dir = $( 'body' ).is( '.rtl' ) ? 'rtl' : 'ltr'; + map = map[dir]; } - conditions = map[dir][profile.name]; + // Check over each browser condition to determine if we are running in a compatible client + if ( typeof map !== 'object' || map[profile.name] === undefined ) { + // Not found, return true if exactMatchOnly not set, false otherwise + return !exactMatchOnly; + } + conditions = map[profile.name]; if ( conditions === false ) { + // Match no versions return false; } + if ( conditions === null ) { + // Match all versions + return true; + } for ( i = 0; i < conditions.length; i++ ) { op = conditions[i][0]; val = conditions[i][1]; diff --git a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js index dc259feee3..277dbf6f6e 100644 --- a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js +++ b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js @@ -16,7 +16,15 @@ * @constructor */ ve.init.mw.ViewPageTarget = function VeInitMwViewPageTarget() { - var currentUri = new mw.Uri( window.location.toString() ); + var browserWhitelisted, + browserBlacklisted, + currentUri = new mw.Uri( window.location.toString() ), + supportsStrictMode = ( function () { + 'use strict'; + return this === undefined; + }() ), + // TODO: MW test runner fires before document.body exists, so we just skip it here. + supportsContentEditable = document.body && 'contentEditable' in document.body; // Parent constructor ve.init.mw.Target.call( @@ -61,10 +69,6 @@ ve.init.mw.ViewPageTarget = function VeInitMwViewPageTarget() { mw.config.get( 'wgAction' ) === 'view' && currentUri.query.diff === undefined ); - this.canBeActivated = ( - $.client.test( ve.init.mw.ViewPageTarget.compatibility ) || - 'vewhitelist' in currentUri.query - ); this.originalDocumentTitle = document.title; this.editSummaryByteLimit = 255; // Tab layout. @@ -72,6 +76,15 @@ ve.init.mw.ViewPageTarget = function VeInitMwViewPageTarget() { // * replace: Re-creates #ca-edit for VisualEditor and adds #ca-editsource. this.tabLayout = 'replace'; + browserWhitelisted = ( + $.client.test( ve.init.mw.ViewPageTarget.compatibility, null, true ) || + 'vewhitelist' in currentUri.query + ); + browserBlacklisted = ( + $.client.test( ve.init.mw.ViewPageTarget.compatibility, null, true ) || + 'veblacklist' in currentUri.query + ); + // Events this.connect( this, { 'load': 'onLoad', @@ -87,7 +100,11 @@ ve.init.mw.ViewPageTarget = function VeInitMwViewPageTarget() { } ); // Initialization - if ( this.canBeActivated ) { + if ( supportsStrictMode && supportsContentEditable && !this.browserBlacklisted ) { + if ( !this.browserWhitelisted || true ) { + // show warning + this.localNoticeMessages.push( 'visualeditor-browserwarning' ); + } if ( currentUri.query.venotify ) { // The following messages can be used here: // visualeditor-notification-saved @@ -128,27 +145,22 @@ ve.inheritClass( ve.init.mw.ViewPageTarget, ve.init.mw.Target ); * @property */ ve.init.mw.ViewPageTarget.compatibility = { - // Left-to-right languages - ltr: { - msie: [['>=', 9]], - firefox: [['>=', 11]], - iceweasel: [['>=', 10]], - safari: [['>=', 5]], - chrome: [['>=', 19]], - opera: false, - netscape: false, - blackberry: false + // The key is the browser name returned by jQuery.client + // The value is either null (match all versions) or a list of tuples + // containing an inequality (<,>,<=,>=) and a version number + whitelist: { + 'msie': [['>=', 9]], + 'firefox': [['>=', 11]], + 'iceweasel': [['>=', 10]], + 'safari': [['>=', 5]], + 'chrome': [['>=', 19]] }, - // Right-to-left languages - rtl: { - msie: [['>=', 9]], - firefox: [['>=', 11]], - iceweasel: [['>=', 10]], - safari: [['>=', 5]], - chrome: [['>=', 19]], - opera: false, - netscape: false, - blackberry: false + blacklist: { + 'msie': [['<', 9]], + 'android': [['<', 3]], + // Blacklist all versions: + 'opera': null, + 'blackberry': null } }; diff --git a/modules/ve/init/mw/ve.init.mw.Target.js b/modules/ve/init/mw/ve.init.mw.Target.js index c0931f17e7..69e84ff34f 100644 --- a/modules/ve/init/mw/ve.init.mw.Target.js +++ b/modules/ve/init/mw/ve.init.mw.Target.js @@ -48,6 +48,8 @@ ve.init.mw.Target = function VeInitMwTarget( $container, pageName, revision ) { this.startTimeStamp = null; this.doc = null; this.editNotices = null; + this.remoteNotices = []; + this.localNoticeMessages = []; this.isMobileDevice = ( 'ontouchstart' in window || ( window.DocumentTouch && document instanceof window.DocumentTouch ) @@ -132,8 +134,7 @@ ve.inheritClass( ve.init.mw.Target, ve.init.Target ); * @emits loadError */ ve.init.mw.Target.onLoad = function ( response ) { - var key, tmp, el, - data = response ? response.visualeditor : null; + var data = response ? response.visualeditor : null; if ( !data && !response.error ) { ve.init.mw.Target.onLoadError.call( @@ -159,36 +160,7 @@ ve.init.mw.Target.onLoad = function ( response ) { this.originalHtml = data.content; this.doc = ve.createDocumentFromHTML( this.originalHtml ); - /* Don't show notices with no visible html (bug 43013). */ - - // Since we're going to parse them, we might as well save these nodes - // so we don't have to parse them again later. - this.editNotices = {}; - - // This is a temporary container for parsed notices in the . - // We need the elements to be in the DOM in order for stylesheets to apply - // and jquery.visibleText to determine whether a node is visible. - tmp = document.createElement( 'div' ); - - // The following is essentially display none, but we can't use that - // since then all descendants will be considered invisible too. - tmp.style.cssText = 'position: static; top: 0; width: 0; height: 0; border: 0; visibility: hidden'; - - document.body.appendChild( tmp ); - for ( key in data.notices ) { - el = $( '
' ) - .addClass( 've-init-mw-viewPageTarget-toolbar-editNotices-notice' ) - .attr( 'rel', key ) - .html( data.notices[key] ) - .get( 0 ); - - tmp.appendChild( el ); - if ( $.getVisibleText( el ).trim() !== '' ) { - this.editNotices[key] = el; - } - tmp.removeChild( el ); - } - document.body.removeChild( tmp ); + this.remoteNotices = data.notices; this.baseTimeStamp = data.basetimestamp; this.startTimeStamp = data.starttimestamp; @@ -197,6 +169,54 @@ ve.init.mw.Target.onLoad = function ( response ) { } }; +/** + * Handle the edit notices being ready for rendering. + * + * @static + * @method + */ +ve.init.mw.Target.onNoticesReady = function() { + var i, len, noticeHtmls, tmp, el; + + // Since we're going to parse them, we might as well save these nodes + // so we don't have to parse them again later. + this.editNotices = {}; + + /* Don't show notices without visible html (bug 43013). */ + + // This is a temporary container for parsed notices in the . + // We need the elements to be in the DOM in order for stylesheets to apply + // and jquery.visibleText to determine whether a node is visible. + tmp = document.createElement( 'div' ); + + // The following is essentially display none, but we can't use that + // since then all descendants will be considered invisible too. + tmp.style.cssText = 'position: static; top: 0; width: 0; height: 0; border: 0; visibility: hidden;'; + document.body.appendChild( tmp ); + + // Merge locally and remotely generated notices + noticeHtmls = this.remoteNotices; + for ( i = 0, len = this.localNoticeMessages.length; i < len; i++ ) { + noticeHtmls.push( + '

' + ve.init.platform.getParsedMessage( this.localNoticeMessages[i] ) + '

' + ); + } + + for ( i = 0, len = noticeHtmls.length; i < len; i++ ) { + el = $( '
' ) + .addClass( 've-init-mw-viewPageTarget-toolbar-editNotices-notice' ) + .html( noticeHtmls[i] ) + .get( 0 ); + + tmp.appendChild( el ); + if ( $.getVisibleText( el ).trim() !== '' ) { + this.editNotices[i] = el; + } + tmp.removeChild( el ); + } + document.body.removeChild( tmp ); +}; + /** * Handle response to a successful edit token refresh request. * @@ -238,6 +258,9 @@ ve.init.mw.Target.onToken = function ( response ) { * @emits load */ ve.init.mw.Target.onReady = function () { + // We need to wait until onReady as local notices may require special messages + ve.init.mw.Target.onNoticesReady.call( this ); + this.loading = false; this.emit( 'load', this.doc ); };