Skip to content

Commit

Permalink
fix(focus-trap): exception when element contains SVG on IE (#3432)
Browse files Browse the repository at this point in the history
Fixes an exception being thrown by the focus trap, if it contains an SVG element on IE. The problem was that on IE, SVG elements don't have the `children` property, which means that we have to fall back to `childNodes`.

Fixes #3410.
  • Loading branch information
crisbeto authored and tinayuangao committed Mar 7, 2017
1 parent 842896b commit d06ad75
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 7 deletions.
36 changes: 35 additions & 1 deletion src/lib/core/a11y/focus-trap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ describe('FocusTrap', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [FocusTrapDirective, FocusTrapWithBindings, SimpleFocusTrap, FocusTrapTargets],
declarations: [
FocusTrapDirective,
FocusTrapWithBindings,
SimpleFocusTrap,
FocusTrapTargets,
FocusTrapWithSvg
],
providers: [InteractivityChecker, Platform, FocusTrapFactory]
});

Expand Down Expand Up @@ -112,6 +118,20 @@ describe('FocusTrap', () => {
expect(document.activeElement.id).toBe('last');
});
});

describe('special cases', () => {
it('should not throw when it has a SVG child', () => {
let fixture = TestBed.createComponent(FocusTrapWithSvg);

fixture.detectChanges();

let focusTrapInstance = fixture.componentInstance.focusTrapDirective.focusTrap;

expect(() => focusTrapInstance.focusFirstTabbableElement()).not.toThrow();
expect(() => focusTrapInstance.focusLastTabbableElement()).not.toThrow();
});
});

});


Expand Down Expand Up @@ -156,3 +176,17 @@ class FocusTrapWithBindings {
class FocusTrapTargets {
@ViewChild(FocusTrapDirective) focusTrapDirective: FocusTrapDirective;
}


@Component({
template: `
<div cdkTrapFocus>
<svg xmlns="http://www.w3.org/2000/svg">
<circle cx="100" cy="100" r="100"/>
</svg>
</div>
`
})
class FocusTrapWithSvg {
@ViewChild(FocusTrapDirective) focusTrapDirective: FocusTrapDirective;
}
22 changes: 16 additions & 6 deletions src/lib/core/a11y/focus-trap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,15 @@ export class FocusTrap {
return root;
}

// Iterate in DOM order.
let childCount = root.children.length;
for (let i = 0; i < childCount; i++) {
let tabbableChild = this._getFirstTabbableElement(root.children[i] as HTMLElement);
// Iterate in DOM order. Note that IE doesn't have `children` for SVG so we fall
// back to `childNodes` which includes text nodes, comments etc.
let children = root.children || root.childNodes;

for (let i = 0; i < children.length; i++) {
let tabbableChild = children[i].nodeType === Node.ELEMENT_NODE ?
this._getFirstTabbableElement(children[i] as HTMLElement) :
null;

if (tabbableChild) {
return tabbableChild;
}
Expand All @@ -147,8 +152,13 @@ export class FocusTrap {
}

// Iterate in reverse DOM order.
for (let i = root.children.length - 1; i >= 0; i--) {
let tabbableChild = this._getLastTabbableElement(root.children[i] as HTMLElement);
let children = root.children || root.childNodes;

for (let i = children.length - 1; i >= 0; i--) {
let tabbableChild = children[i].nodeType === Node.ELEMENT_NODE ?
this._getLastTabbableElement(children[i] as HTMLElement) :
null;

if (tabbableChild) {
return tabbableChild;
}
Expand Down

0 comments on commit d06ad75

Please sign in to comment.