Skip to content

Commit

Permalink
fix(aio): remove all links from toc titles (angular#22533)
Browse files Browse the repository at this point in the history
The previous approach just removed the first `a` tag that
was found, but now that the header-link anchor is not at
the start of the heading, it could fail.

Closes angular#22493

PR Close angular#22533
  • Loading branch information
petebacondarwin authored and leo6104 committed Mar 25, 2018
1 parent 7eed6e2 commit 2e7f0a8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 19 deletions.
28 changes: 16 additions & 12 deletions aio/src/app/shared/toc.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,23 +291,20 @@ describe('TocService', () => {
});
});

describe('TocItem for an h2 with anchor link and extra whitespace', () => {
describe('TocItem for an h2 with links and extra whitespace', () => {
let docId: string;
let docEl: HTMLDivElement;
let tocItem: TocItem;
let expectedTocContent: string;

beforeEach(() => {
docId = 'fizz/buzz/';
expectedTocContent = 'Setup to develop <i>locally</i>.';

// An almost-actual <h2> ... with extra whitespace
docEl = callGenToc(`
callGenToc(`
<h2 id="setup-to-develop-locally">
<a href="tutorial/toh-pt1#setup-to-develop-locally" aria-hidden="true">
Setup to <a href="moo">develop</a> <i>locally</i>.
<a class="header-link" href="tutorial/toh-pt1#setup-to-develop-locally" aria-hidden="true">
<span class="icon icon-link"></span>
</a>
${expectedTocContent}
</h2>
`, docId);

Expand All @@ -331,7 +328,7 @@ describe('TocService', () => {
it('should have bypassed HTML sanitizing of heading\'s innerHTML ', () => {
const domSanitizer: TestDomSanitizer = injector.get(DomSanitizer);
expect(domSanitizer.bypassSecurityTrustHtml)
.toHaveBeenCalledWith(expectedTocContent);
.toHaveBeenCalledWith('Setup to develop <i>locally</i>.');
});
});
});
Expand All @@ -352,13 +349,20 @@ class TestDomSanitizer {
}

class MockScrollSpyService {
$lastInfo: {
private $$lastInfo: {
active: Subject<ScrollItem | null>,
unspy: jasmine.Spy
};
unspy: jasmine.Spy,
} | undefined;

get $lastInfo() {
if (!this.$$lastInfo) {
throw new Error('$lastInfo is not yet defined. You must call `spyOn` first.');
}
return this.$$lastInfo;
}

spyOn(headings: HTMLHeadingElement[]): ScrollSpyInfo {
return this.$lastInfo = {
return this.$$lastInfo = {
active: new Subject<ScrollItem | null>(),
unspy: jasmine.createSpy('unspy'),
};
Expand Down
24 changes: 17 additions & 7 deletions aio/src/app/shared/toc.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export interface TocItem {
export class TocService {
tocList = new ReplaySubject<TocItem[]>(1);
activeItemIndex = new ReplaySubject<number | null>(1);
private scrollSpyInfo: ScrollSpyInfo | null;
private scrollSpyInfo: ScrollSpyInfo | null = null;

constructor(
@Inject(DOCUMENT) private document: any,
Expand Down Expand Up @@ -53,15 +53,25 @@ export class TocService {

// This bad boy exists only to strip off the anchor link attached to a heading
private extractHeadingSafeHtml(heading: HTMLHeadingElement) {
const a = this.document.createElement('a') as HTMLAnchorElement;
a.innerHTML = heading.innerHTML;
const anchorLink = a.querySelector('a');
if (anchorLink) {
a.removeChild(anchorLink);
const div: HTMLDivElement = this.document.createElement('div');
div.innerHTML = heading.innerHTML;
const anchorLinks: NodeListOf<HTMLAnchorElement> = div.querySelectorAll('a');
for (let i = 0; i < anchorLinks.length; i++) {
const anchorLink = anchorLinks[i];
if (!anchorLink.classList.contains('header-link')) {
// this is an anchor that contains actual content that we want to keep
// move the contents of the anchor into its parent
const parent = anchorLink.parentNode!;
while (anchorLink.childNodes.length) {
parent.insertBefore(anchorLink.childNodes[0], anchorLink);
}
}
// now remove the anchor
anchorLink.remove();
}
// security: the document element which provides this heading content
// is always authored by the documentation team and is considered to be safe
return this.domSanitizer.bypassSecurityTrustHtml(a.innerHTML.trim());
return this.domSanitizer.bypassSecurityTrustHtml(div.innerHTML.trim());
}

private findTocHeadings(docElement: Element): HTMLHeadingElement[] {
Expand Down

0 comments on commit 2e7f0a8

Please sign in to comment.