-
Notifications
You must be signed in to change notification settings - Fork 10k
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
[api-minor] Include the document /Lang attribute in the textContent-data #17941
[api-minor] Include the document /Lang attribute in the textContent-data #17941
Conversation
de07ea9
to
6dd2dd8
Compare
4d811e1
to
0ac822e
Compare
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/eabbc1b8aad8045/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/e959d8976999b5e/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/e959d8976999b5e/output.txt Total script time: 27.60 mins
Image differences available at: http://54.241.84.105:8877/e959d8976999b5e/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/eabbc1b8aad8045/output.txt Total script time: 40.41 mins
Image differences available at: http://54.193.163.58:8877/eabbc1b8aad8045/reftest-analyzer.html#web=eq.log |
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.
Looks good to me, with the two questions below answered/addressed. Thanks!
0ac822e
to
6588c30
Compare
/botio-linux unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @timvandermeij received. Current queue size: 1 Live output at: http://54.241.84.105:8877/88e047ea12f3d16/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/88e047ea12f3d16/output.txt Total script time: 2.62 mins
|
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.
r=me, with passing Windows tests once the bot is working again. Thank you for the patch!
- These changes will allow a simpler way of implementing PR 17770. - The /Lang attribute is fetched lazily, with the first `getTextContent` invocation. Given the existing worker-thread caching, this will thus only need to be done *once* per PDF document (and most PDFs don't included this data). - This makes the /Lang attribute *directly available* in the `textLayer`, which has the following advantages: - We don't need to block, and thus delay, overall viewer initialization on fetching it (nor pass it around throughout the viewer). - Third-party users of the `textLayer` will automatically benefit from this, once we start actually using the /Lang attribute in PR 17770. *Please note:* This also, importantly, means that the `text` reference-tests will then cover this code (which wouldn't otherwise have been the case).
6588c30
to
6d523c3
Compare
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/af6b465e78d0e4d/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/cebfb7f73a0d0af/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/cebfb7f73a0d0af/output.txt Total script time: 27.43 mins
Image differences available at: http://54.241.84.105:8877/cebfb7f73a0d0af/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/af6b465e78d0e4d/output.txt Total script time: 40.22 mins
Image differences available at: http://54.193.163.58:8877/af6b465e78d0e4d/reftest-analyzer.html#web=eq.log |
These changes will allow a simpler way of implementing PR Add language attribute to canvas #17770.
The /Lang attribute is fetched lazily, with the first
getTextContent
invocation. Given the existing worker-thread caching, this will thus only need to be done once per PDF document (and most PDFs don't included this data).This makes the /Lang attribute directly available in the
textLayer
, which has the following advantages:We don't need to block, and thus delay, overall viewer initialization on fetching it (nor pass it around throughout the viewer).
Third-party users of the
textLayer
will automatically benefit from this, once we start actually using the /Lang attribute in PR Add language attribute to canvas #17770.Please note: This also, importantly, means that the
text
reference-tests will then cover this code (which wouldn't otherwise have been the case).