Skip to content

Conversation

@robertleeplummerjr
Copy link

Added the ability to use ajax, which can overcome certain security deficiencies found in Chrome and IE.

Overcomes the following obstacle: http://stackoverflow.com/questions/19244006/chrome-basic-auth-image-requests-return-401-but-not-when-called-directly

This is now a browser behaviour, where other browsers are following suite.

index.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be an else, right? Otherwise this both loads images via URL and loads them with ajax slightly after.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an implied else, with the return; at the bottom of the if statement.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I corrected the else to me more explicit. Also I fixed a bug that caused onload to be called multiple times depending on latency and I also now cache the images internally to cut down on requests.

use else because it looks better
correct onload duplicate callback issue causing images to intermittently show up
@robertleeplummerjr
Copy link
Author

Is this pr satisfactory? I'd like to get it merged and start on handling canvas layers so that, for example, the points here: http://robertleeplummerjr.github.io/Leaflet.glify/ show up. I plan on adding this today.

@robertleeplummerjr
Copy link
Author

added canvas layer rendering

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is there a settimeout here, and previously?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't work without it. Honestly not sure.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is always that chance that it had something to do with the gravitation and alignment of the planets... I'll try it later without it just to make sure.

:P

@tmcw
Copy link
Contributor

tmcw commented Jun 7, 2016

Happy to merge once the setTimeout is deciphered.

im.src = cache;
if (!loaded) {
setTimeout(function() {
im.onload();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image isn't yet added to the page, but is base64. setTimeout(fn, 0) ensures it is loaded when the next available context is, which (since we just set the src) is just after the src is rendered. Otherwise the image is not yet rendered, thus resulting in a blank image.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the tardy response.

@robertleeplummerjr
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants