Skip to content

Commit

Permalink
fix(platform-browser): DomEventsPlugin should always be the last pl…
Browse files Browse the repository at this point in the history
…ugin to be called for `supports()`.

This fixes the issues when `BrowserModule` is not the first module imported.

Fixes #37149 #37850
  • Loading branch information
JeanMeche committed May 21, 2023
1 parent 87b8aba commit 60a97de
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 18 deletions.
8 changes: 7 additions & 1 deletion goldens/public-api/platform-browser/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export function enableDebugTools<T>(ref: ComponentRef<T>): ComponentRef<T>;
export const EVENT_MANAGER_PLUGINS: InjectionToken<EventManagerPlugin[]>;

// @public
export class EventManager {
export class EventManager implements EventManagerInterface {
constructor(plugins: EventManagerPlugin[], _zone: NgZone);
addEventListener(element: HTMLElement, eventName: string, handler: Function): Function;
getZone(): NgZone;
Expand All @@ -92,6 +92,12 @@ export class EventManager {
static ɵprov: i0.ɵɵInjectableDeclaration<EventManager>;
}

// @public (undocumented)
export interface EventManagerInterface {
addEventListener(element: HTMLElement, eventName: string, handler: Function): Function;
getZone(): NgZone;
}

// @public
export const HAMMER_GESTURE_CONFIG: InjectionToken<HammerGestureConfig>;

Expand Down
2 changes: 1 addition & 1 deletion packages/platform-browser/src/dom/events/dom_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {DOCUMENT} from '@angular/common';
import {Inject, Injectable} from '@angular/core';

import {EventManagerPlugin} from './event_manager';
import {EventManagerPlugin} from './interface';

@Injectable()
export class DomEventsPlugin extends EventManagerPlugin {
Expand Down
26 changes: 13 additions & 13 deletions packages/platform-browser/src/dom/events/event_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {Inject, Injectable, InjectionToken, NgZone, ɵRuntimeError as RuntimeErr

import {RuntimeErrorCode} from '../../errors';

import {DomEventsPlugin} from './dom_events';
import {EventManagerInterface, EventManagerPlugin} from './interface';

/**
* The injection token for the event-manager plug-in service.
*
Expand All @@ -26,7 +29,7 @@ export const EVENT_MANAGER_PLUGINS =
* @publicApi
*/
@Injectable()
export class EventManager {
export class EventManager implements EventManagerInterface {
private _plugins: EventManagerPlugin[];
private _eventNameToPlugin = new Map<string, EventManagerPlugin>();

Expand All @@ -37,7 +40,15 @@ export class EventManager {
plugins.forEach((plugin) => {
plugin.manager = this;
});
this._plugins = plugins.slice().reverse();

const otherPlugins = plugins.filter(p => !(p instanceof DomEventsPlugin));
this._plugins = otherPlugins.slice().reverse();

// DomEventsPlugin.supports() always returns true, it should always be the last plugin.
const domEventPlugin = plugins.find(p => p instanceof DomEventsPlugin);
if (domEventPlugin) {
this._plugins.push(domEventPlugin);
}
}

/**
Expand Down Expand Up @@ -81,14 +92,3 @@ export class EventManager {
return plugin;
}
}

export abstract class EventManagerPlugin {
constructor(private _doc: any) {}

// Using non-null assertion because it's set by EventManager's constructor
manager!: EventManager;

abstract supports(eventName: string): boolean;

abstract addEventListener(element: HTMLElement, eventName: string, handler: Function): Function;
}
3 changes: 2 additions & 1 deletion packages/platform-browser/src/dom/events/hammer_gestures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import {DOCUMENT} from '@angular/common';
import {Inject, Injectable, InjectionToken, NgModule, Optional, Provider, ɵConsole as Console} from '@angular/core';

import {EVENT_MANAGER_PLUGINS, EventManagerPlugin} from './event_manager';
import {EVENT_MANAGER_PLUGINS} from './event_manager';
import {EventManagerPlugin} from './interface';



Expand Down
41 changes: 41 additions & 0 deletions packages/platform-browser/src/dom/events/interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {NgZone} from '@angular/core';

/**
* @publicApi
*/
export interface EventManagerInterface {
/**
* Registers a handler for a specific element and event.
*
* @param element The HTML element to receive event notifications.
* @param eventName The name of the event to listen for.
* @param handler A function to call when the notification occurs. Receives the
* event object as an argument.
* @returns A callback function that can be used to remove the handler.
*/
addEventListener(element: HTMLElement, eventName: string, handler: Function): Function;

/**
* Retrieves the compilation zone in which event listeners are registered.
*/
getZone(): NgZone;
}

export abstract class EventManagerPlugin {
constructor(private _doc: any) {}

// Using non-null assertion because it's set by EventManager's constructor
manager!: EventManagerInterface;

abstract supports(eventName: string): boolean;

abstract addEventListener(element: HTMLElement, eventName: string, handler: Function): Function;
}
2 changes: 1 addition & 1 deletion packages/platform-browser/src/dom/events/key_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
import {Inject, Injectable, NgZone} from '@angular/core';

import {EventManagerPlugin} from './event_manager';
import {EventManagerPlugin} from './interface';

/**
* Defines supported modifiers for key events.
Expand Down
1 change: 1 addition & 0 deletions packages/platform-browser/src/platform-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export {disableDebugTools, enableDebugTools} from './browser/tools/tools';
export {By} from './dom/debug/by';
export {REMOVE_STYLES_ON_COMPONENT_DESTROY} from './dom/dom_renderer';
export {EVENT_MANAGER_PLUGINS, EventManager} from './dom/events/event_manager';
export {EventManagerInterface} from './dom/events/interface';
export {HAMMER_GESTURE_CONFIG, HAMMER_LOADER, HammerGestureConfig, HammerLoader, HammerModule} from './dom/events/hammer_gestures';
export {DomSanitizer, SafeHtml, SafeResourceUrl, SafeScript, SafeStyle, SafeUrl, SafeValue} from './security/dom_sanitization_service';
export {HydrationFeature, provideClientHydration, HydrationFeatureKind, withNoDomReuse, withNoHttpTransferCache} from './hydration';
Expand Down
43 changes: 42 additions & 1 deletion packages/platform-browser/test/dom/events/event_manager_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
*/

import {ɵgetDOM as getDOM} from '@angular/common';
import {Component, destroyPlatform, Inject, NgModule, provideZoneChangeDetection} from '@angular/core';
import {NgZone} from '@angular/core/src/zone/ng_zone';
import {TestBed} from '@angular/core/testing';
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import {BrowserModule} from '@angular/platform-browser/public_api';
import {DomEventsPlugin} from '@angular/platform-browser/src/dom/events/dom_events';
import {EventManager, EventManagerPlugin} from '@angular/platform-browser/src/dom/events/event_manager';
import {EVENT_MANAGER_PLUGINS, EventManager, EventManagerPlugin} from '@angular/platform-browser/src/dom/events/event_manager';

import {createMouseEvent, el} from '../../../testing/src/browser_util';

Expand Down Expand Up @@ -487,6 +491,43 @@ describe('EventManager', () => {
done();
});
});

it('should always order the plugins with DomEventsPlugin being last', (done) => {
destroyPlatform();
const app = getDOM().createElement('app', doc);
doc.body.appendChild(app);
class Plugin {}
@NgModule({
providers: [
{
provide: EVENT_MANAGER_PLUGINS,
useClass: Plugin,
multi: true,
},
],
})
class EventModule {
}

@Component({
selector: 'app',
template: '',
})
class MyComp {
constructor(EventManager: EventManager) {
const plugins = (EventManager as any)._plugins;
expect(plugins[1]).toBeInstanceOf(Plugin);
expect(plugins[plugins.length - 1]).toBeInstanceOf(DomEventsPlugin);
done();
}
}

@NgModule({imports: [EventModule, BrowserModule], bootstrap: [MyComp], declarations: [MyComp]})
class SomeModule {
}

platformBrowserDynamic().bootstrapModule(SomeModule);
});
});
})();

Expand Down

0 comments on commit 60a97de

Please sign in to comment.