-
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
[Editor] Add support for printing newly added FreeText annotations #15082
Conversation
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 comments addressed; thank you!
src/core/annotation.js
Outdated
static async createNewAppearanceStream( | ||
annotation, | ||
xref, | ||
/* params */ _unused |
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.
How about simply leaving it as follows, since defining a different but likewise unused variable here seems unnecessary?
/* params */ _unused | |
params |
const ap = await this.createNewAppearanceStream(annotation, xref, params); | ||
const annotationDict = this.createNewDict(annotation, xref, { ap }); | ||
|
||
return new this.prototype.constructor({ |
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.
That's an interesting, and clever, way to avoid duplicating code!
test/unit/test_utils.js
Outdated
@@ -79,6 +79,7 @@ class XRefMock { | |||
this._map = Object.create(null); | |||
this.stats = new DocStats({ send: () => {} }); | |||
this._newRefNum = null; | |||
this.stream = { isDataLoaded: true }; |
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.
What change(s) made this necessary?
Also, please let's avoid any future issues by providing a "valid" BaseStream
-instance here:
this.stream = { isDataLoaded: true }; | |
this.stream = new NullStream(); |
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.
In the test:
https://github.com/mozilla/pdf.js/pull/15082/files#diff-63129533f0fcab1395066c5eecb394e84cc3538b5e016a2f0bb49942825610e6R4088
we have to load the resources:
Line 857 in d72a85f
return objectLoader.load().then(function () { |
and the
isDataLoaded
is used:pdf.js/src/core/object_loader.js
Line 66 in d72a85f
if (this.xref.stream.isDataLoaded) { |
fa2d692
to
30c63eb
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/5b7c5c83a12c861/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/38fdb4335b5fb63/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/5b7c5c83a12c861/output.txt Total script time: 26.35 mins
Image differences available at: http://54.241.84.105:8877/5b7c5c83a12c861/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/38fdb4335b5fb63/output.txt Total script time: 28.40 mins
Image differences available at: http://54.193.163.58:8877/38fdb4335b5fb63/reftest-analyzer.html#web=eq.log |
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/11263bafac365c4/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/ca0716bf15c5597/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/ca0716bf15c5597/output.txt Total script time: 22.07 mins
|
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/11263bafac365c4/output.txt Total script time: 22.61 mins
|
No description provided.