[api-minor] Allow loading pdf fonts into another document. #12154
Conversation
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/b96a294e5c3f3ff/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/df07274dde84095/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/b96a294e5c3f3ff/output.txt Total script time: 26.65 mins
Image differences available at: http://54.67.70.0:8877/b96a294e5c3f3ff/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/df07274dde84095/output.txt Total script time: 30.85 mins
Image differences available at: http://54.215.176.217:8877/df07274dde84095/reftest-analyzer.html#web=eq.log |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/eddab0368bc48bc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/0dc901d21f68b5a/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/eddab0368bc48bc/output.txt Total script time: 26.83 mins
Image differences available at: http://54.67.70.0:8877/eddab0368bc48bc/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/0dc901d21f68b5a/output.txt Total script time: 29.47 mins
Image differences available at: http://54.215.176.217:8877/0dc901d21f68b5a/reftest-analyzer.html#web=eq.log |
There was a problem hiding this comment.
Looks good to me with these final comments addressed and a rebase. r? @Snuffleupagus for a final check to make sure all is fine now. Thanks!
|
rebased:) |
|
Gentle ping @Snuffleupagus if you have time for a final check to make sure I didn't miss anything. |
Sorry about the delay here, I've been swamped with other things! Unfortunately I still believe that #12154 (comment) applies here, even in the latest version of the patch, since https://github.com/mozilla/pdf.js/pull/12154/files#diff-348626e82475ec3536e620f4e5508f75R117-R118 looks quite cryptic and it's not (at least to me) immediately obvious what's actually happing there without very careful reading of the code. @timvandermeij What's your opinion on this specific part of the unit-tests, since I worry that this admittedly clever but very terse checker function may hurt maintainability of these tests? |
|
@Snuffleupagus I'm struggling to understand. Is there a builtin assertion I could use? How is this cryptic? We just want to verify the set or list has the item matching our pattern? |
|
@Snuffleupagus I've updated to add a custom matcher which makes the callsite look like this: expect(ownerDocument.fonts).toContainElementMatching(checkFont);The custom matcher is here: a32ec02#diff-348626e82475ec3536e620f4e5508f75R22-R54 const collectionMatches = (iterable, matcherFunction) =>
Array.from(iterable).find(matcherFunction) !== undefined; |
|
Oof, that's indeed a lot more code for something that's not that difficult. Compared to what it was in the first iteration I think const collectionMatches = (iterable, matcherFunction) =>
Array.from(iterable).find(matcherFunction) !== undefined;was the most clear. Maybe it would help to not make it a variable like this but just inline the check? So: Array.from(ownerDocument.fonts).find(checkFont) !== undefinedEven then I don't really see anything wrong with |
|
Suggestion incorporated and rebased. |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/6bacf6d11ac635d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/285a1d5c7c98dc8/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/6bacf6d11ac635d/output.txt Total script time: 26.74 mins
Image differences available at: http://54.67.70.0:8877/6bacf6d11ac635d/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/285a1d5c7c98dc8/output.txt Total script time: 31.66 mins
Image differences available at: http://54.215.176.217:8877/285a1d5c7c98dc8/reftest-analyzer.html#web=eq.log |
|
Thank you! |
|
@timvandermeij What does the release schedule look like? |
|
We don't have a fixed release schedule. I believe every week or so the code is merged into Firefox Nightly so the builds get tested by users, and NPM releases are usually made when a number of |
|
I look forward for the next npm release of pdfjs-dist 😊 |
Addresses PR #12131 browser test failures.
Fixes #12152