Skip to content

Commit

Permalink
fix(aio): only remove the header-link from toc titles
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
  • Loading branch information
petebacondarwin committed Feb 28, 2018
1 parent 9eaf1bb commit 7c69b2e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 13 deletions.
13 changes: 6 additions & 7 deletions aio/src/app/shared/toc.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,23 +291,22 @@ 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>.';
expectedTocContent = 'Setup to <a href="moo">develop</a> <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">
${expectedTocContent}
<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 @@ -325,7 +324,7 @@ describe('TocService', () => {
it('should have removed anchor link from tocItem html content', () => {
expect((<TestSafeHtml>tocItem.content)
.changingThisBreaksApplicationSecurity)
.toEqual('Setup to develop <i>locally</i>.');
.toEqual('Setup to <a href="moo">develop</a> <i>locally</i>.');
});

it('should have bypassed HTML sanitizing of heading\'s innerHTML ', () => {
Expand Down
12 changes: 6 additions & 6 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,15 @@ 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');
const div = this.document.createElement('div') as HTMLAnchorElement;
div.innerHTML = heading.innerHTML;
const anchorLink = div.querySelector('a.header-link');
if (anchorLink) {
a.removeChild(anchorLink);
div.removeChild(anchorLink);
}
// 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 7c69b2e

Please sign in to comment.