Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable automatic URL linking (bug 1019475) #19110

Merged
merged 1 commit into from
Feb 6, 2025
Merged

Enable automatic URL linking (bug 1019475) #19110

merged 1 commit into from
Feb 6, 2025

Conversation

ryzokuken
Copy link
Collaborator

@ryzokuken ryzokuken commented Nov 26, 2024 •
edited
Loading

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:

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 26, 2024
View reviewed changes
web/pdf_page_view.js Fixed Show fixed Hide fixed
Snuffleupagus
Snuffleupagus requested changes Nov 26, 2024
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this perform, especially in documents that contain a lot of text?

Also, we probably want a new option/preference to be able to disable this functionality.

web/pdf_page_view.js Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
web/annotation_layer_builder.js Outdated Show resolved Hide resolved
@ryzokuken ryzokuken force-pushed the autolink-demo branch 2 times, most recently from a7d0024 to f3737b5 Compare December 10, 2024 23:11
github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 10, 2024
View reviewed changes
web/autolinker.js Fixed Show fixed Hide fixed
@ryzokuken ryzokuken force-pushed the autolink-demo branch 2 times, most recently from cad781f to 93e5417 Compare December 10, 2024 23:16
@ryzokuken
Copy link
Collaborator Author

@Snuffleupagus

Besides, it isn't necessary since the textContent is already available once the textLayer has rendered; see

I was a bit unsure if I understood exactly what you were suggesting but how does this commit look? It "fetches" the textContents from the previous render of the textLayer and makes the processing step sync.

93e5417

@ryzokuken
Copy link
Collaborator Author

@Snuffleupagus nvm my last comment, I figured it out after looking at pdfPageView._textHighlighter.textDivs a couple of times it occurred to me what you were talking about.

https://github.com/mozilla/pdf.js/pull/19110/files#diff-71772f56be799df522c2076ab5fa476253ef15c607af5812536c41696d97cd59R73

@ryzokuken ryzokuken marked this pull request as ready for review December 16, 2024 22:10
@calixteman calixteman changed the title Enable automatic URL linking Enable automatic URL linking (bug 1019475) Jan 7, 2025
@marco-c marco-c linked an issue Jan 7, 2025 that may be closed by this pull request
Enable automatic URL hyperlinking #3172
Closed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 8, 2025
View reviewed changes
web/autolinker.js Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 14, 2025
View reviewed changes
web/autolinker.js Fixed Show fixed Hide fixed
nicolo-ribaudo
nicolo-ribaudo reviewed Jan 14, 2025
View reviewed changes
web/autolinker.js Outdated Show resolved Hide resolved
@ryzokuken ryzokuken force-pushed the autolink-demo branch 4 times, most recently from 864c116 to 6aee919 Compare January 16, 2025 12:06
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 16, 2025
View reviewed changes
web/autolinker.js Fixed Show resolved Hide resolved
calixteman
calixteman reviewed Jan 16, 2025
View reviewed changes
web/autolinker.js Outdated Show resolved Hide resolved
@ryzokuken
Copy link
Collaborator Author

At a glance, the integration test failures seem unrelated to this PR, although I'll check to be sure.

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/5a2fadaeaeba323/output.txt

Total script time: 59.50 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

calixteman
calixteman approved these changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

@calixteman
Copy link
Contributor

@Snuffleupagus: gentle ping for your approval.
We'd like to merge it asap in nightly.
In considering your last comment I guess it's fine, but I prefer asking.

Snuffleupagus
Snuffleupagus approved these changes Feb 3, 2025
View reviewed changes
src/display/annotation_layer.js Outdated Show resolved Hide resolved
web/autolinker.js Outdated Show resolved Hide resolved
extensions/chromium/preferences_schema.json Outdated Show resolved Hide resolved
web/pdf_page_view.js Outdated Show resolved Hide resolved
@calixteman
Copy link
Contributor

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/8269e46056099cb/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/6d7147e11acd620/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/6d7147e11acd620/output.txt

Total script time: 29.31 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/8269e46056099cb/output.txt

Total script time: 58.75 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@ryzokuken
Enable automatic URL linking
61ba1ea 
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.
@calixteman
Copy link
Contributor

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/a575219a2cf1a69/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_test from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/45df2f91f2a7971/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/a575219a2cf1a69/output.txt

Total script time: 29.44 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/45df2f91f2a7971/output.txt

Total script time: 59.44 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@calixteman
Copy link
Contributor

The integration tests failures are unrelated.
@Snuffleupagus gentle ping for the review, thank you.

@calixteman calixteman merged commit 2d773cb into mozilla:master Feb 6, 2025
17 checks passed
@timvandermeij timvandermeij mentioned this pull request Feb 9, 2025
Open
This was referenced Feb 10, 2025
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo left review comments

@Snuffleupagus Snuffleupagus Snuffleupagus approved these changes

@calixteman calixteman calixteman approved these changes

Assignees

@ryzokuken ryzokuken

Labels
viewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable automatic URL hyperlinking
6 participants
@ryzokuken @calixteman @Snuffleupagus @moz-tools-bot @nicolo-ribaudo @timvandermeij