Enable automatic URL linking #19110
Enable automatic URL linking #19110
Conversation
Automatically detect links in the text content of a file and automatically generate link annotations at the appropriate locations to achieve automatic link detection and hyperlinking.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| #processLinks() { | ||
| return this.pdfPage.getTextContent().then(content => { | ||
| const [text, diffs] = normalizedTextContent(content); | ||
| const urlRegex = /\b(?:https?:\/\/|mailto:|www.)(?:[[\S--\[]--\p{P}]|\/|[\p{P}--\[]+[[\S--\[]--\p{P}])+/gmv; |
Check notice
Code scanning / CodeQL
Syntax error Note
| } | ||
|
|
||
| #processLinks() { | ||
| return this.pdfPage.getTextContent().then(content => { |
There was a problem hiding this comment.
We absolutely cannot fetch the textContent twice for each rendered page, since that'll be really inefficient in general.
Besides, it isn't necessary since the textContent is already available once the textLayer has rendered; see
pdf.js/web/text_layer_builder.js
Line 99 in 079eb24
pdf.js/web/text_highlighter.js
Lines 47 to 59 in 079eb24
| const urlRegex = /\b(?:https?:\/\/|mailto:|www.)(?:[[\S--\[]--\p{P}]|\/|[\p{P}--\[]+[[\S--\[]--\p{P}])+/gmv; | ||
| const matches = text.matchAll(urlRegex); |
There was a problem hiding this comment.
A regular expression isn't sufficient to guarantee that valid URLs are found, so please use createValidAbsoluteUrl to ensure that every "candidate" is actually correct/safe.
| #processLinks() { | ||
| return this.pdfPage.getTextContent().then(content => { | ||
| const [text, diffs] = normalizedTextContent(content); | ||
| const urlRegex = /\b(?:https?:\/\/|mailto:|www.)(?:[[\S--\[]--\p{P}]|\/|[\p{P}--\[]+[[\S--\[]--\p{P}])+/gmv; |
There was a problem hiding this comment.
Also, the regular expression should probably be created just once (and then cached) to avoid re-creating it for every page.
| annotationType: 2, | ||
| annotationFlags: 4, |
There was a problem hiding this comment.
These should use actual constants, rather than hard-coded numbers.
| isEditable: false, | ||
| hasApperance: false, | ||
| modificationDate: null, | ||
| structParent: 2, |
There was a problem hiding this comment.
This should probably use the default-value instead?
| structParent: 2, | |
| structParent: -1, |
| // NOTE everything from here on is arbitrary | ||
| borderStyle: { | ||
| width: 2, | ||
| rawWidth: 2, | ||
| style: 1, | ||
| dashArray: [3], | ||
| horizontalCornerRadius: 0, | ||
| verticalCornerRadius: 0 |
There was a problem hiding this comment.
If these don't matter, please at least make sure that you pick values that correspond to the default ones in AnnotationBorderStyle.
| annotations.push(...linkAnnotations); | ||
|
|
There was a problem hiding this comment.
Have you actually tested this with PDFs that already contain Annotations?
Because it appears to me that this could easily end up creating "duplicate" and overlapping LinkAnnotations in many cases.
For an initial implementation we might want to either:
- Don't use these if the page already has any Annotations.
- Don't use these if the page already has LinkAnnotations.
Automatically detect links in the text content of a file and automatically generate link annotations at the appropriate locations to achieve automatic link detection and hyperlinking.
References:
Please note that this is a WIP PR for soliciting your feedback while I work on polishing things and hopefully optimizing further.