Acceptance criteria
- We want to discourage access to certain jQuery and window to encourage behaviours that are compatible with modern browser developments
- We want to reduce the possibility of side effects e.g, component SearchOverlay can impact TalkOverlay
- We want to make it easier to test components and change their behaviour in different contexts.
- We want to prevent us using jQuery/window directly in future coding. We'll enforce using eslint and mw.log.deprecate
Global use of jQuery for finding elements
Many of our classes access global jQuery to select elements. We talked about how this was an anti-pattern as using a global query can access elements outside the View.
ag "[^\.]\\$\( (?!(function|window|this|'<|document|'html|'body))" resources/ --ignore resources/**/[^a-z]*
Note: Regex issue. Should not match any filename which begin with lowercase letter...
Expected
- Only View.js should do this...
- Do not access 'body', 'html' from inside classes - move these outside the classes.
Global use of window, document
docfix x ~/git/core/extensions/MobileFrontend $ ag window resources/
resources/mobile.categories.overlays/CategoryAddOverlay.js
125: window.location.hash = '#/categories';
resources/mobile.editor.common/EditorOverlayBase.js
65: this.allowCloseWindow = mw.confirmCloseWindow( {
165: !window.confirm( mw.msg( 'mobile-frontend-editor-new-page-confirm', mw.user ) )
205: $( window ).off( 'beforeunload.mfeditorwarning' );
213: window.location = mw.util.getUrl( title );
216: window.location.reload();
241: window.scrollTo( 0, 1 );
319: self.allowCloseWindow.release();
324: this.allowCloseWindow.release();
resources/mobile.editor.overlay/EditorOverlay.js
155: if ( window.confirm( mw.msg( 'mobile-frontend-editor-switch-confirm' ) ) ) {
293: window.scrollTo( 0, this.scrollTop );
457: window.location = mw.util.getUrl( title );
resources/mobile.editor.ve/ve.init.mw.MobileFrontendArticleTarget.js
30: this.onWindowScrollDebounced = ve.debounce( this.onWindowScroll.bind( this ), 100 );
31: $( this.getElementWindow() ).on( 'scroll', this.onWindowScrollDebounced );
52: $( this.getElementWindow() ).off( 'scroll', this.onWindowScrollDebounced );
82: * Handle window scroll events
84:ve.init.mw.MobileFrontendArticleTarget.prototype.onWindowScroll = function () {
85: var $window, windowTop, contentTop,
88: // The window can only scroll in iOS if the keyboard has been opened
90: // iOS applies a scroll offset to the window to move the cursor
92: $window = $( target.getElementWindow() );
93: windowTop = $window.scrollTop();
96: $window.scrollTop( 0 );
97: surface.scrollTo( contentTop + windowTop );
247: window.history.back();
resources/mobile.editor.ve/VisualEditorOverlay.js
149:}( mw.mobileFrontend, jQuery, window.ve ) );
resources/mobile.infiniteScroll/InfiniteScroll.js
66: * Listen to scroll on window and notify this._onScroll
112: var scrollBottom = $( window ).scrollTop() + $( window ).height(),
resources/mobile.mediaViewer/ImageGateway.js
38: imageSizeMultiplier = ( window.devicePixelRatio && window.devicePixelRatio > 1 ) ? window.devicePixelRatio : 1;
51: iiurlwidth: findSizeBucket( $( window ).width() * imageSizeMultiplier ),
52: iiurlheight: findSizeBucket( $( window ).height() * imageSizeMultiplier )
resources/mobile.mediaViewer/ImageOverlay.js
86: window.location.hash = '#/media/' + encodeURIComponent( thumbnail.getFileName() );
205: window.location.hash = '';
215: * Fit the image into the window if its dimensions are bigger than the window dimensions.
216: * Compare window width to height ratio to that of image width to height when setting
222: var detailsHeight, windowWidth, windowHeight, windowRatio, $img;
226: windowWidth = $( window ).width();
227: windowHeight = $( window ).height() - detailsHeight;
228: windowRatio = windowWidth / windowHeight;
231: if ( this.imgRatio > windowRatio ) {
232: if ( windowWidth < this.thumbWidth ) {
234: width: windowWidth,
239: if ( windowHeight < this.thumbHeight ) {
242: height: windowHeight
250: * Function to adjust the height of details section to not more than 50% of window height.
254: var windowHeight = $( window ).height();
255: if ( this.$( '.details' ).height() > windowHeight * 0.50 ) {
256: this.$( '.details' ).css( 'max-height', windowHeight * 0.50 );
resources/mobile.nearby/Nearby.js
218: hash = window.location.hash;
232: window.location.hash = $( this ).attr( 'id' );
238: offset = $( window.location.hash ).offset();
241: $( window ).scrollTop( offset.top );
resources/mobile.notifications.overlay/NotificationsOverlay.js
163: window.location.href = this.badge.getNotificationURL();
resources/mobile.patrol.ajax/init.js
55: // (or open it in a new window, bypassing this ajax module).
resources/mobile.pointerOverlay/PointerOverlay.js
137: // use a global window event so that on resizes it is correctly positioned.
resources/mobile.references/references.less
14: max-height: 400px; // or half of window height, whichever is smaller (see ReferencesDrawerBeta.js)
resources/mobile.references/ReferencesDrawer.js
59: var windowHeight = $( window ).height();
64: if ( windowHeight / 2 < 400 ) {
65: this.$el.css( 'max-height', windowHeight / 2 );
104: window.location.hash = id;
resources/mobile.search/SearchOverlay.js
125: * SearchOverlay is not managed by OverlayManager and using window.history.back() causes
135: window.location.hash = '';
168: window.history.back();
180: window.setTimeout( function () {
237: window.location.href = $link.attr( 'href' );
resources/mobile.search/SearchOverlay.less
53: // See https://msdn.microsoft.com/en-us/library/windows/apps/hh465740.aspx (T102325)
resources/mobile.startup/browser.js
115: return window.innerWidth >= val || window.innerHeight >= val;
159: return 'ontouchstart' in window;
177: browser = new Browser( window.navigator.userAgent, $( 'html' ) );
resources/mobile.startup/Drawer.js
70: $window = $( window );
72: $window.one( 'click.drawer', $.proxy( self, 'hide' ) );
74: $window.one( 'scroll.drawer', $.proxy( self, 'hide' ) );
85: $( window ).off( '.drawer' );
resources/mobile.startup/Overlay.js
9: $window = $( window );
12: * Mobile modal window
148: window.history.back();
196: window.scrollTo( 0, 1 );
205: $window
210: self._resizeContent( $window.height() );
227: window.scrollTo( document.body.scrollLeft, this.scrollTop );
233: $window.off( '.ios' );
246: * Fit the overlay content height to the window taking overlay header and footer heights
250: * @param {number} windowHeight The height of the window
252: _resizeContent: function ( windowHeight ) {
254: windowHeight -
279: this._resizeContent( $( window ).height() );
288: $window.scrollTop( 999 );
289: keyboardHeight = $window.scrollTop();
290: $window.scrollTop( 0 );
293: if ( $window.height() > keyboardHeight ) {
294: self._resizeContent( $window.height() - keyboardHeight );
299: self._resizeContent( $window.height() );
301: $window.scrollTop( 0 );
resources/mobile.startup/Page.js
56: * @cfg {string} defaults.hash Window location hash.
74: hash: window.location.hash,
238: $( window ).on( 'load', function () {
239: window.location.hash = self.options.hash;
resources/mobile.startup/PageList.js
46: window.setTimeout( function () {
resources/mobile.startup/Skin.js
60: * Tests current window size and if suitable loads styles and scripts specific for larger devices
173: offset = $( window ).height() * 1.5,
resources/mobile.talk.overlays/TalkOverlay.js
111: window.location = mw.util.getUrl( options.title );
resources/mobile.talk.overlays/TalkSectionAddOverlay.js
72: if ( this._saveHit || empty || window.confirm( confirmMessage ) ) {
107: window.history.back();
resources/mobile.toggle/toggle.js
216: window.scrollTo( 0, $target.offset().top );
314: var hash = window.location.hash;
332: window.location.hash = internalRedirectHash;Hygiene: move window.location manipulation to clients
Possible violators may be identified with rg -sg\!/tests window.location. They seem to fall mostly into two categories:
- Bound directly or almost directly (click or click-like event listener, simple function call, etc):
- resources/mobile.nearby/Nearby._postRenderLinks() (in a private click handler)
- resources/mobile.toggle/toggle.Toggle()
- resources/mobile.categories.overlays/CategoryAddOverlay.onSaveClick()
- resources/mobile.mediaViewer/ImageOverlay.setNewImage()
- resources/mobile.references/ReferencesDrawer.showReference()
It seems that these cases should generally be straightforward to solve by having the client pass a click listener.
- Bound to longer event chain:
- resources/mobile.search/SearchOverlay.onExit() and onClickResult() (the latter is wired into the router)
- resources/mobile.mediaViewer/ImageOverlay.onExit()
- resources/mobile.editor.common/EditorOverlayBase.onSaveComplete()
- resources/mobile.talk.overlays/TalkOverlay.postRender()
- resources/mobile.editor.overlay/EditorOverlay.onSaveBegin()
- resources/mobile.startup/Page.postRender()
- resources/mobile.notifications.overlay/NotificationsOverlay.onError()
For #2, emit an event to the client or refactor to #1. e.g., in TalkOverlay, the caller creates a new instance and the postRender hook kicks off an HTTP request that may fail:
this.pageGateway.getPage( options.title ).fail( function ( resp ) {
...
window.location = ...
...
} )...Here we can either emit on the failure to the client or refactor the HTTP request to be initiated by the client and return a Deferred.