Skip to content

Commit

Permalink
fix(tabs): infinite loop when selectedIndex is set to NaN (#2389)
Browse files Browse the repository at this point in the history
  • Loading branch information
crisbeto authored and kara committed Jan 6, 2017
1 parent 99963c4 commit f4cfc2d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
17 changes: 13 additions & 4 deletions src/lib/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ describe('MdTabGroup', () => {
fixture.detectChanges();
expect(component.selectedIndex).toBe(2);
});

it('should not crash when setting the selected index to NaN', () => {
let component = fixture.debugElement.componentInstance;

expect(() => {
component.selectedIndex = NaN;
fixture.detectChanges();
}).not.toThrow();
});
});

describe('dynamic binding tabs', () => {
Expand Down Expand Up @@ -244,19 +253,19 @@ describe('MdTabGroup', () => {
* Checks that the `selectedIndex` has been updated; checks that the label and body have their
* respective `active` classes
*/
function checkSelectedIndex(index: number, fixture: ComponentFixture<any>) {
function checkSelectedIndex(expectedIndex: number, fixture: ComponentFixture<any>) {
fixture.detectChanges();

let tabComponent: MdTabGroup = fixture.debugElement
.query(By.css('md-tab-group')).componentInstance;
expect(tabComponent.selectedIndex).toBe(index);
expect(tabComponent.selectedIndex).toBe(expectedIndex);

let tabLabelElement = fixture.debugElement
.query(By.css(`.md-tab-label:nth-of-type(${index + 1})`)).nativeElement;
.query(By.css(`.md-tab-label:nth-of-type(${expectedIndex + 1})`)).nativeElement;
expect(tabLabelElement.classList.contains('md-tab-label-active')).toBe(true);

let tabContentElement = fixture.debugElement
.query(By.css(`md-tab-body:nth-of-type(${index + 1})`)).nativeElement;
.query(By.css(`md-tab-body:nth-of-type(${expectedIndex + 1})`)).nativeElement;
expect(tabContentElement.classList.contains('md-tab-body-active')).toBe(true);
}

Expand Down
6 changes: 4 additions & 2 deletions src/lib/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ export class MdTabGroup {
* a new selected tab should transition in (from the left or right).
*/
ngAfterContentChecked(): void {
// Clamp the next selected index to the bounds of 0 and the tabs length.
// Clamp the next selected index to the bounds of 0 and the tabs length. Note the `|| 0`, which
// ensures that values like NaN can't get through and which would otherwise throw the
// component into an infinite loop (since Math.max(NaN, 0) === NaN).
this._indexToSelect =
Math.min(this._tabs.length - 1, Math.max(this._indexToSelect, 0));
Math.min(this._tabs.length - 1, Math.max(this._indexToSelect || 0, 0));

// If there is a change in selected index, emit a change event. Should not trigger if
// the selected index has not yet been initialized.
Expand Down

0 comments on commit f4cfc2d

Please sign in to comment.