mozilla :: #pdfjs

 
25 Jul 2017
06:30nish17yury: please review my pull request https://github.com/mozilla/pdf.js/pull/8690
17:33yurybdahl: updated #8627
17:33soakbotPull 8627: Fallback on font widths if CFF data is broken. https://github.com/mozilla/pdf.js/pull/8627
17:33yuryhi
18:11mukulyury: Hi, we are failing 2 unit tests.
18:12yuryany ideas?
18:12mukulIn this: https://github.com/mukulmishra18/pdf.js/blob/network-streaming/test/unit/api_spec.js#L64, I see loadingTask.onProgress is not invoked.
18:13yurywhat did we change in terms of onProgress
18:13mukulyury: I see here: https://github.com/mukulmishra18/pdf.js/blob/network-streaming/src/display/network.js#L464 `this.onProgress` is null.
18:14mukulWe are setting onProgress when `headersReady` is resolved.
18:15mukulInstead of doing `if (this.onProgress) {...`, if we do `this._headersReceivedCapability.promise.then(() => {...)`, then test is passing.
18:15yuryso we may loose some reports
18:15mukulyes right. Because we are checking if onProgress is set or not.
18:16mukulyury: If it is not set than progress report is ignored.
18:18yurymukul: we probably need to bind some onProgress at 'GetReader'
18:18yuryand save it somewhere locally
18:19yuryand at headersReady send what we got so far
18:19yurywe just need last progress
18:21mukul`last progress`? What that mean, sorry I didn't understand.
18:23yurywhatever will be reported at in callback you set at GetReader
18:27mukulyury: So that mean we will save `evt` of onProgress in `GetReader` call, something like `fullReader.onProgress = (evt) => { lastProgress = evt };` and send this `lastProgress` in `ReaderHeadersReady` call?
18:27yurysomething like that, yes. I would make copy of evt tought
18:30mukulyury: making copy means `lastProgress = Object.create(evt);` ?
18:30yurylastProgress = { total: evt.total, ... };
18:45mukulyury: It is passing now.
18:45yuryawesome
18:46mukulI think for the second failure we needs to add check for `isCancelled` in `error` too.
18:47mukulsomething like: https://github.com/mukulmishra18/pdf.js/blob/network-streaming/src/shared/util.js#L1457, in error() below.
18:47yuryokay
19:10mukulyury: For test this: https://github.com/mukulmishra18/pdf.js/blob/network-streaming/test/unit/api_spec.js#L178 to pass, `sink.error` must be called.
19:12mukulAdding extra check for `this.isCancelled` inhibits `error` message to send.
19:12yurymukul: so we are cancelling too early? what is canceling it?
19:16mukulyury: I think loadingTask is cancelled by rejection of PDFWorker promise.
19:22yurymukul: we need to know what's cancelling it before we deliver the error
19:23mukulfullReader.read() operation fails and calls sink.error
19:23yuryright, but why isCancelled is set
19:25mukulyury: in network.js, I see read() operation fails if it has _storedError, that is set when _onError is called.
19:27nish17yury: review --> https://github.com/mozilla/pdf.js/pull/8690
19:56mukulyury: I see 2 `error` calls, so probably it is not `cancel` that is deleting `streamController`, rather `error` is deleting it.
19:57yurysounds plausible
20:09mukulyury: Don't know why but when I am checking for this.isCancelled in the `error` method of sink, it does not send `error` message even once.
20:13github_pdfjs[pdf.js] timvandermeij pushed 2 new commits to master: https://github.com/mozilla/pdf.js/compare/bd8c12119a34...44a5cec25eea
20:13github_pdfjspdf.js/master d14956d Apoorv Mishra: Fix TypeError that occurs in colorspace.js on accidentally passing an 'Array' instead of 'TypedArray'...
20:13github_pdfjspdf.js/master 44a5cec Tim van der Meij: Merge pull request #8666 from apoorv-mishra/fix-colorspace...
20:13soakbotPull 8666: Fix TypeError that occurs in colorspace.js on accidentally passing an 'Array' instead of 'TypedArray'. https://github.com/mozilla/pdf.js/pull/8666
20:20yurymukul: something bad is happening then
20:23mukulyury: can you please help me? I am having difficulty in understanding what wrong is happening.
20:33yurymukul: if it's intermittent, compare paths when it's good
20:33yurymoar logs
20:35github_pdfjs[pdf.js] timvandermeij pushed 2 new commits to master: https://github.com/mozilla/pdf.js/compare/44a5cec25eea...37ac8f8623be
20:35github_pdfjspdf.js/master 23ec6b1 Jonas Jenwald: Add a fallback for non-embedded SegoeUISymbol font (issue 8697)...
20:35github_pdfjspdf.js/master 37ac8f8 Tim van der Meij: Merge pull request #8698 from Snuffleupagus/issue-8697...
20:35soakbotPull 8698: Add a fallback for non-embedded SegoeUISymbol font (issue 8697). https://github.com/mozilla/pdf.js/pull/8698
20:42mukulyury: When I am logging `this` inside error() method it is giving me `undefined` for both two times.
20:46github_pdfjs[pdf.js] pdfjsbot force-pushed gh-pages from e8d9b14 to 867444f: https://github.com/mozilla/pdf.js/commits/gh-pages
20:46github_pdfjspdf.js/gh-pages 867444f pdfjsbot: gh-pages site created via gulpfile.js script...
 
25 Jul 2017
   
Last message: 1 minute and 41 seconds ago