New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed setOpacity in IE7 and IE8 #1371

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@javisantana

setOpacity is not working in IE7 and IE8

you can check it in the following link (changes the opacity each 2 seconds). It uses the leaflet 0.5 hosted in the CDN (I tested with the current master and i get the same behavior)

example: http://bl.ocks.org/d/4942923/
code: https://gist.github.com/javisantana/4942923

this is the version with the patch applied

example: http://bl.ocks.org/d/4942985/
code: https://gist.github.com/javisantana/4942985

I'm not pretty sure if that is the best way to manage the opacity for IE7 since the opacity for all the tiles should be changed to work.

@javisantana

This comment has been minimized.

Show comment
Hide comment
@javisantana

javisantana Feb 13, 2013

forget this pull request from now, looks like native IE8 still fails (compatiblity mode works). Working in the fix

forget this pull request from now, looks like native IE8 still fails (compatiblity mode works). Working in the fix

@javisantana

This comment has been minimized.

Show comment
Hide comment
@javisantana

javisantana Feb 13, 2013

last commit fixes native ie7 and native ie8

last commit fixes native ie7 and native ie8

@danzel

This comment has been minimized.

Show comment
Hide comment
@danzel

danzel Feb 13, 2013

Member

The setOpacity support for old IE has always been a contentious issue :-(
AFAIK setOpacity works in IE7 and IE8?
Opacity is definitely broken for tile layers in IE (which you've fixed with your TileLayer changes) - see #1084

These 2 issues have some background on why setOpacity is like it is:
#667
#827

I think you could achieve this fix with just the changes to TileLayer? (Haven't tested yet sorry, my IE VMs are hosed)

Member

danzel commented Feb 13, 2013

The setOpacity support for old IE has always been a contentious issue :-(
AFAIK setOpacity works in IE7 and IE8?
Opacity is definitely broken for tile layers in IE (which you've fixed with your TileLayer changes) - see #1084

These 2 issues have some background on why setOpacity is like it is:
#667
#827

I think you could achieve this fix with just the changes to TileLayer? (Haven't tested yet sorry, my IE VMs are hosed)

@javisantana

This comment has been minimized.

Show comment
Hide comment
@javisantana

javisantana Feb 14, 2013

@danzel

setOpacity is not working in IE7 and IE8 (both native), you can check it with this simple example: http://bl.ocks.org/d/4942923/

In IE8 opacity worked but only the first time.

I've changed only TiledLayer, the other changes were to identify IE8 and to fix setOpacity in DomUtils (was not working neither, check it with the debugger in IE8), but I suppose I could do it. I'm not very familiar with Leaflet source code so maybe there is a better way to do it.

@danzel

setOpacity is not working in IE7 and IE8 (both native), you can check it with this simple example: http://bl.ocks.org/d/4942923/

In IE8 opacity worked but only the first time.

I've changed only TiledLayer, the other changes were to identify IE8 and to fix setOpacity in DomUtils (was not working neither, check it with the debugger in IE8), but I suppose I could do it. I'm not very familiar with Leaflet source code so maybe there is a better way to do it.

@danzel

This comment has been minimized.

Show comment
Hide comment
@danzel

danzel Feb 14, 2013

Member

I'll take a look at IE 7/8 opacity today. Thanks heaps for these changes, they are awesome :-)

IE7/8 setOpacity works for markers, it should just be TileLayers that are broken.
http://bl.ocks.org/d/83ee04165b1fab0c9a41/

Member

danzel commented Feb 14, 2013

I'll take a look at IE 7/8 opacity today. Thanks heaps for these changes, they are awesome :-)

IE7/8 setOpacity works for markers, it should just be TileLayers that are broken.
http://bl.ocks.org/d/83ee04165b1fab0c9a41/

@javisantana

This comment has been minimized.

Show comment
Hide comment
@javisantana

javisantana Feb 14, 2013

Yeah, when I wrote setOpacity i was refering to TileLayer setOpacity.

Tomorrow I will fix TileLayer.setOpacity and will revert the DomUtil.setOpacity

Thansks for your help

El 14/02/2013, a las 22:18, Dave Leaver notifications@github.com escribió:

I'll take a look at IE 7/8 opacity today. Thanks heaps for these changes, they are awesome :-)

IE7/8 setOpacity works for markers, it should just be TileLayers that are broken.
http://bl.ocks.org/d/83ee04165b1fab0c9a41/


Reply to this email directly or view it on GitHub.

Yeah, when I wrote setOpacity i was refering to TileLayer setOpacity.

Tomorrow I will fix TileLayer.setOpacity and will revert the DomUtil.setOpacity

Thansks for your help

El 14/02/2013, a las 22:18, Dave Leaver notifications@github.com escribió:

I'll take a look at IE 7/8 opacity today. Thanks heaps for these changes, they are awesome :-)

IE7/8 setOpacity works for markers, it should just be TileLayers that are broken.
http://bl.ocks.org/d/83ee04165b1fab0c9a41/


Reply to this email directly or view it on GitHub.

@danzel

This comment has been minimized.

Show comment
Hide comment
@danzel

danzel Feb 14, 2013

Member

Awesome, sounds good!

I've done a cludge up over here:
https://github.com/danzel/Leaflet/tree/oldie-fixes
Which demonstrates about what I think we want. The changes in there (which is just your TileLayer changes tidied up a bit) fix tileLayer setOpacity in IE 7/8 in my testing.

Member

danzel commented Feb 14, 2013

Awesome, sounds good!

I've done a cludge up over here:
https://github.com/danzel/Leaflet/tree/oldie-fixes
Which demonstrates about what I think we want. The changes in there (which is just your TileLayer changes tidied up a bit) fix tileLayer setOpacity in IE 7/8 in my testing.

@javisantana

This comment has been minimized.

Show comment
Hide comment
@javisantana

javisantana Feb 15, 2013

@danzel I just tested your branch and it works for IE7 and IE8 (both native) using the map-mobile.html test so i think we could close this and merge yours.

@danzel I just tested your branch and it works for IE7 and IE8 (both native) using the map-mobile.html test so i think we could close this and merge yours.

@mourner

This comment has been minimized.

Show comment
Hide comment
@mourner

mourner Feb 15, 2013

Member

Nice work guys! Dave, pull it in!

Member

mourner commented Feb 15, 2013

Nice work guys! Dave, pull it in!

@mourner mourner closed this Feb 15, 2013

danzel added a commit to danzel/Leaflet that referenced this pull request Feb 15, 2013

@danzel

This comment has been minimized.

Show comment
Hide comment
@danzel

danzel Feb 15, 2013

Member

Great, have pulled in.
Thanks for fixing this @javisantana :-)

Member

danzel commented Feb 15, 2013

Great, have pulled in.
Thanks for fixing this @javisantana :-)

@mourner

This comment has been minimized.

Show comment
Hide comment
@mourner

mourner Feb 15, 2013

Member

Thanks Dave and Javi :)

Member

mourner commented Feb 15, 2013

Thanks Dave and Javi :)

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