box / viewer.js

A viewer for documents converted with the Box View API
Apache License 2.0
336 stars 99 forks source link

IE crashes when we load SVG assets #160

Open harish278 opened 9 years ago

harish278 commented 9 years ago

viewer js error

Got this error in Internet explorer 11 (running on laptop with i5 processor, 4gb ram) . I was loading only svg assets from https://www.dropbox.com/sh/9s1xvbsnukx6u70/AAC3lAt_9xkiTfcPkwGM8Jfba?dl=0 .

viewer = Crocodoc.createViewer(element, {
   url: magazinePath,
   layout: Crocodoc.LAYOUT_VERTICAL_SINGLE_COLUMN,
   enableTextSelection: true,
   enableDragging: false,
   useWindowAsViewport: true
});
viewer.load();
scope.currentPage = 1;

viewer.on('ready', function (event) {
   scope.$parent.viewerNumPages = event.data.numPages;
});

viewer.on('pagefocus', function(page) {
   scope.$apply(function() {
      scope.$parent.viewerCurrentPage = page.data.page;
   });
});
lakenen commented 9 years ago

@harish278 I can't access the dropbox link to reproduce this issue. Could you share a downloadable View API session or host the file somewhere else? Thanks.

harish278 commented 9 years ago

You can download the file from:

lakenen commented 9 years ago

@harish278 please just send the original pdf to api@box.com

peetersn commented 9 years ago

@harish278 @lakenen Sent the PDF.

harish278 commented 9 years ago

@lakenen Is there anything else that we can do specifically to help you with reproduction of this issue?

lakenen commented 9 years ago

@harish278 I was able to reproduce the issue. IE is apparently running out of memory, perhaps because of the number of large images in the document. It can be resolved by unloading the pages fully. There's unfortunately no exposed option in viewer.js for forcing this behavior, but I could patch the library to unload pages fully in IE 11 (currently this only happens in mobile and IE < 10).

peetersn commented 9 years ago

@lakenen This is good news (that you can reproduce the issue that was reported). As far as we are concerned, we want to make sure that we have stability on mobile (rather than get Desktop IE to work). We would welcome this patch. Exposing this option would actually allow us to tweak the behaviour for every kind of browser.

peetersn commented 9 years ago

@lakenen I don't want to sound pushy, but do you have any estimates when we can expect this? This is still a production issue for us at the moment and I would like to be able to give some kind of resolution to it. If it's not possible at the moment, please also let me know, so I can explain that. Thanks!!

lakenen commented 9 years ago

@peetersn sorry about the delays here. I'm nervous about exposing this as an external config, because it's very low level and could change in the future. Instead, I'm probably going to refactor this code a bit such that it'll be possible to write a plugin that modifies this behavior. This could still break with updates to viewer.js in the future, but it makes me feel better knowing that it's not published as an external API.

In the mean time, you could test for IE and force PNG representations.

harish278 commented 9 years ago

@lakenen,

  1. Having PNG assets along with the SVG makes the size of the whole file (with both PNG and SVG assets) increase considerably.
  2. Couldn't have only PNG assets, as the quality was not as good as SVG assets in high end devices.
  3. Tried having Crocodoc.getUtility('support').svg = false for IE which worked, but couldn't have in production due to problem with size (point 1).
if (window.navigator.userAgent.indexOf('MSIE ') || window.navigator.userAgent.indexOf('Trident/')) {
    Crocodoc.getUtility('support').svg = false;
}
xgibster commented 9 years ago

Has there been any progress with resolving this? We are having similar problems with running out memory in IE 11 when using the viewer.

harish278 commented 9 years ago

@xgibster No, waiting for @lakenen's code refactoring.