-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix: Hyperlinks in PDF not showing correctly when printing the radar #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,9 @@ | ||
| const { constructSheetUrl, getDocumentOrSheetId, getSheetName } = require('../../src/util/urlUtils') | ||
| const { | ||
| constructSheetUrl, | ||
| getDocumentOrSheetId, | ||
| getSheetName, | ||
| convertRelativeUrlsToAbsolute, | ||
| } = require('../../src/util/urlUtils') | ||
| const queryParams = require('../../src/util/queryParamProcessor') | ||
|
|
||
| jest.mock('../../src/util/queryParamProcessor') | ||
|
|
@@ -74,4 +79,73 @@ describe('Url Utils', () => { | |
|
|
||
| expect(sheetName).toEqual('sheetName') | ||
| }) | ||
|
|
||
| describe('convertRelativeUrlsToAbsolute', () => { | ||
| beforeEach(() => { | ||
| delete window.location | ||
| window.location = Object.create(window) | ||
| window.location.origin = 'https://radar.thoughtworks.com' | ||
| window.location.protocol = 'https:' | ||
| }) | ||
|
|
||
| it('should return empty string for empty input', () => { | ||
| expect(convertRelativeUrlsToAbsolute('')).toEqual('') | ||
| }) | ||
|
|
||
| it('should return null for null input', () => { | ||
| expect(convertRelativeUrlsToAbsolute(null)).toEqual(null) | ||
| }) | ||
|
|
||
| it('should return undefined for undefined input', () => { | ||
| expect(convertRelativeUrlsToAbsolute(undefined)).toEqual(undefined) | ||
| }) | ||
|
|
||
| it('should not modify absolute URLs', () => { | ||
| const html = '<a href="https://example.com/page">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(html) | ||
| }) | ||
|
|
||
| it('should not modify mailto links', () => { | ||
| const html = '<a href="mailto:test@example.com">Email</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(html) | ||
| }) | ||
|
|
||
| it('should not modify tel links', () => { | ||
| const html = '<a href="tel:+1234567890">Call</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(html) | ||
| }) | ||
|
|
||
| it('should convert root-relative URLs to absolute', () => { | ||
| const html = '<a href="/radar/Techniques/continuous-delivery">CD</a>' | ||
| const expected = '<a href="https://radar.thoughtworks.com/radar/Techniques/continuous-delivery">CD</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
|
|
||
| it('should convert relative URLs to absolute', () => { | ||
| const html = '<a href="page/subpage">Link</a>' | ||
| const expected = '<a href="https://radar.thoughtworks.com/page/subpage">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
|
|
||
| it('should convert protocol-relative URLs', () => { | ||
| const html = '<a href="//example.com/page">Link</a>' | ||
| const expected = '<a href="https://example.com/page">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
|
|
||
| it('should handle multiple links in the same HTML', () => { | ||
| const html = | ||
| '<p>Check out <a href="/radar/Techniques/cd">CD</a> and <a href="https://example.com">Example</a></p>' | ||
| const expected = | ||
| '<p>Check out <a href="https://radar.thoughtworks.com/radar/Techniques/cd">CD</a> and <a href="https://example.com">Example</a></p>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
|
|
||
| it('should handle single quotes in href', () => { | ||
| const html = "<a href='/radar/page'>Link</a>" | ||
| // Note: the regex normalizes quotes to double quotes | ||
| const expected = '<a href="https://radar.thoughtworks.com/radar/page">Link</a>' | ||
| expect(convertRelativeUrlsToAbsolute(html)).toEqual(expected) | ||
| }) | ||
| }) | ||
|
||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,8 +24,39 @@ function getSheetName() { | |||||
| return queryParams.sheetName | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Converts relative URLs in HTML content to absolute URLs. | ||||||
| * This is useful for PDF printing where relative URLs like "/radar/..." | ||||||
| * need to show the full URL. | ||||||
| * @param {string} html - HTML content potentially containing relative URLs | ||||||
| * @returns {string} HTML with relative URLs converted to absolute | ||||||
| */ | ||||||
| function convertRelativeUrlsToAbsolute(html) { | ||||||
| if (!html) return html | ||||||
|
|
||||||
| const baseUrl = window.location.origin | ||||||
|
|
||||||
| // Convert relative href attributes to absolute URLs | ||||||
| return html.replace( | ||||||
| /href=["'](?!https?:\/\/|mailto:|tel:)([^"']+)["']/gi, | ||||||
|
||||||
| /href=["'](?!https?:\/\/|mailto:|tel:)([^"']+)["']/gi, | |
| /href=["'](?!https?:\/\/|mailto:|tel:|javascript:|data:)([^"']+)["']/gi, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a test case for
http://URLs (without the 's') to ensure the regex properly handles both HTTP and HTTPS protocols. While the regex patternhttps?correctly handles both, an explicit test would make this behavior clearer and prevent regressions.