Skip to content

Commit

Permalink
virtual-scroll: Avoid using bypassSecurityTrustStyle which is banned …
Browse files Browse the repository at this point in the history
…in google3 (#12181)
  • Loading branch information
mmalerba authored and josephperrott committed Jul 17, 2018
1 parent 4ffb39f commit 1afdaac
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 28 deletions.
3 changes: 1 addition & 2 deletions src/cdk-experimental/scrolling/virtual-scroll-viewport.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
Wrap the rendered content in an element that will be used to offset it based on the scroll
position.
-->
<div #contentWrapper class="cdk-virtual-scroll-content-wrapper"
[style.transform]="_renderedContentTransform">
<div #contentWrapper class="cdk-virtual-scroll-content-wrapper">
<ng-content></ng-content>
</div>
<!--
Expand Down
12 changes: 0 additions & 12 deletions src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,6 @@ describe('CdkVirtualScrollViewport', () => {
viewport = testComponent.viewport;
});

it('should sanitize transform inputs', fakeAsync(() => {
finishInit(fixture);
viewport.orientation = 'arbitrary string as orientation' as any;
viewport.setRenderedContentOffset(
'arbitrary string as offset' as any, 'arbitrary string as to' as any);
fixture.detectChanges();
flush();

expect((viewport._renderedContentTransform as any).changingThisBreaksApplicationSecurity)
.toBe('translateY(NaNpx)');
}));

it('should render initial state', fakeAsync(() => {
finishInit(fixture);

Expand Down
29 changes: 15 additions & 14 deletions src/cdk-experimental/scrolling/virtual-scroll-viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
ViewChild,
ViewEncapsulation,
} from '@angular/core';
import {DomSanitizer, SafeStyle} from '@angular/platform-browser';
import {animationFrameScheduler, fromEvent, Observable, Subject} from 'rxjs';
import {sampleTime, take, takeUntil} from 'rxjs/operators';
import {CdkVirtualForOf} from './virtual-for-of';
Expand Down Expand Up @@ -68,11 +67,8 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
*/
_totalContentSize = 0;

/** The transform used to offset the rendered content wrapper element. */
_renderedContentTransform: SafeStyle;

/** The raw string version of the rendered content transform. */
private _rawRenderedContentTransform: string;
/** The the rendered content transform. */
private _renderedContentTransform: string;

/** The currently rendered range of indices. */
private _renderedRange: ListRange = {start: 0, end: 0};
Expand Down Expand Up @@ -107,8 +103,9 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
/** A list of functions to run after the next change detection cycle. */
private _runAfterChangeDetection: Function[] = [];

constructor(public elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef,
private _ngZone: NgZone, private _sanitizer: DomSanitizer,
constructor(public elementRef: ElementRef,
private _changeDetectorRef: ChangeDetectorRef,
private _ngZone: NgZone,
@Inject(VIRTUAL_SCROLL_STRATEGY) private _scrollStrategy: VirtualScrollStrategy) {}

ngOnInit() {
Expand Down Expand Up @@ -223,17 +220,16 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
let transform = `translate${axis}(${Number(offset)}px)`;
this._renderedContentOffset = offset;
if (to === 'to-end') {
// TODO(mmalerba): The viewport should rewrite this as a `to-start` offset on the next render
// cycle. Otherwise elements will appear to expand in the wrong direction (e.g.
// `mat-expansion-panel` would expand upward).
transform += ` translate${axis}(-100%)`;
// The viewport should rewrite this as a `to-start` offset on the next render cycle. Otherwise
// elements will appear to expand in the wrong direction (e.g. `mat-expansion-panel` would
// expand upward).
this._renderedContentOffsetNeedsRewrite = true;
}
if (this._rawRenderedContentTransform != transform) {
if (this._renderedContentTransform != transform) {
// We know this value is safe because we parse `offset` with `Number()` before passing it
// into the string.
this._rawRenderedContentTransform = transform;
this._renderedContentTransform = this._sanitizer.bypassSecurityTrustStyle(transform);
this._renderedContentTransform = transform;
this._markChangeDetectionNeeded(() => {
if (this._renderedContentOffsetNeedsRewrite) {
this._renderedContentOffset -= this.measureRenderedContentSize();
Expand Down Expand Up @@ -317,6 +313,11 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {

// Apply changes to Angular bindings.
this._ngZone.run(() => this._changeDetectorRef.detectChanges());
// Apply the content transform. The transform can't be set via an Angular binding because
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
// the `Number` function first to coerce it to a numeric value.
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
// Apply the pending scroll offset separately, since it can't be set up as an Angular binding.
if (this._pendingScrollOffset != null) {
if (this.orientation === 'horizontal') {
Expand Down

0 comments on commit 1afdaac

Please sign in to comment.