search

thatcher / openseadragon

This project has moved to its new github organization at github.com/openseadragon, please join us!
http://openseadragon,github.com/
37 stars 14 forks source link

Update src/navigator.js #24

Closed chonselaar2 closed 11 years ago

chonselaar2 commented 11 years ago

Fixes a consistent hard crash in IE7 and 8, when zooming in far (nested DZI's).

Ventero commented 11 years ago

Just out of curiosity: Which line in the try block throws the exception in IE? I don't have IE here to test it myself.

Edit: Ah, I guess this.displayRegion.style is null?

iangilman commented 11 years ago

I'd prefer to address the problem rather than just wrapping it in a try. If this.displayRegion.style is null, let's add a check for that and return (or don't call the function in the first place) if so.

chonselaar2 commented 11 years ago

The style is not null - the issue is that a negative width/height is being calculated and set if the difference between topleft and bottomright is less than 3. IE 7 and 8 do not accept negative widths. I am not sure what this code block or the 3 constant is doing exactly (without looking further into it). An assertion that the width is non-negative would work as well of course.

Ventero commented 11 years ago

How about something like Ventero/openseadragon@b21c369 instead? Though of course it'd be better to figure out why the -3 is necessary at all.

Note that I didn't rebuild openseadragon.js in that commit, as I currently don't have access to a cloned repository, so in case you should merge that commit, you'll have to rebuild it afterwards (actually I think it shouldn't be part of the repository at all, but that discussion should probably happen in a separate bug report).

iangilman commented 11 years ago

@Ventero, that looks great; can you make a pull request out of it? I agree we should have a better explanation for the magic number (I do not)... for now, could you add a // TODO: What's this magic number mean? to the code?

By "it shouldn't be part of the repository" I assume you mean the built openseadragon.js? I'm coming to that conclusion myself; I've started #25 for that purpose.

Ventero commented 11 years ago

Sure, see #26. Added the commend in a separate commit in that pull request.

And yeah, I was talking about the combined openseadragon.js.