[OpenLayers-Dev] getDimensions: whether or not it should update the css display attribute

[Andreas Hocevar]

Hi,

http://trac.openlayers.org/changeset/10125 seems to have caused a regression, and as Bruno stated in http://trac.openlayers.org/ticket/2414#comment:10, it does not fix the reported issue.

So I am in favor of reverting r10125 before releasing OpenLayers 2.9.1.

Also, IRC user quietriver wanted to report this regression on May 1st, but his mail to the dev list did not go through. Here is what he wrote:

Hi all,

The change to Map.js contained in [1] that was included in 2.9 seems to
have broken the code for IE8 (at least under the conditions where the map
size is not specified in the css, in order for it to fill the screen.
I tried setting width and height to "100%", but that doesn't fix it.

I reverted the function getCurrentSize to its previous form shown
below, and everything worked again.

                  getCurrentSize: function() {

                      var size = new OpenLayers.Size(this.div.clientWidth,
                                                     this.div.clientHeight);

                      // Workaround for the fact that hidden
elements return 0 for size.
                      if (size.w == 0 && size.h == 0 ||
isNaN(size.w) && isNaN(size.h)) {
                          var dim =
OpenLayers.Element.getDimensions(this.div);
                          size.w = dim.width;
                          size.h = dim.height;
                      }

                      if (size.w == 0 && size.h == 0 ||
isNaN(size.w) && isNaN(size.h)) {
                          size.w = parseInt(this.div.style.width);
                          size.h = parseInt(this.div.style.height);
                      }

                      return size;
                  }

The issue was that (as the workaround suggests!) that the size being
returned from this.div.clientWidth and this.div.clientHeight was 0.
This meant that without the workaround in place (the first 'if' block
below), the second 'if' block would execute. The this.div.style.width
and this.div.style.height returned empty strings to parseInt, which
set w and h of size to be #QNAN values.

And 2.9 also includes the code:

var newSize = this.getCurrentSize();
if (newSize && !isNaN(newSize.h) && !isNaN(newSize.w)) {

in the updateSize function of map.js.

The result: the NaN values returned by getCurrentSize() cause map.size
not to be set, leaving it at its default null value! Which breaks
everything!

The knock on effect, the first error you get is: size.clone() in
setSize in Renderer.js fails with the error "null is null or not an
object".


I wasn't sure if I was free to submit this as a bug or not, so I've
given you all I know: feel free to ticket it at will.

Openlayers is a fantastic bit of kit! I'll hopefully submit some of
the work I'm doing for you to consider for inclusion when I finish my
current project. Thanks for all your hard work!

Hope this helps,

Nick

[1] http://trac.openlayers.org/attachment/ticket/2414/patch_2414-r10113.3.diff
On Jan 26, 2010, at 09:17 , Eric Lemoine wrote:

> On Thu, Jan 14, 2010 at 11:40 AM, Bruno Binet
> <bruno.binet at camptocamp.com> wrote:
>> Hi devs,
>> 
>> I recently came across a bug in OpenLayers that causes Element
>> getDimensions method to unexpectedly set a display:none on the map
>> div.
>> It occured when updateSize was called on the map while map div was
>> already display:none (from a css class).
>> The ticket #2414 [1] is reporting the problem.
>> 
>> In my opinion, getDimensions should not set a css attribute to get the
>> real dimensions of an element. I don't like the idea of a "get" method
>> to set attributes. If a element is display:none, then getDimensions
>> should return 0.
>> 
>> When looking at svn history, getDimensions comes from Prototype
>> framework, and this method is only used in Map.js in getCurrentSize
>> method, and getCurrentSize is only used in updateSize method and
>> Layer/MapGuide.js (which I think should use map.size instead).
>> I'd support the idea of removing the code which temporary make the
>> element regain its original size, and letting the web application in
>> charge of manually calling updateSize when display:none is removed
>> from the map div.
>> But the current behaviour comes from the beginning of OpenLayers [2],
>> so there may be good reasons for it?
> 
> I don't see any good reason for it. So I agree with having
> getDimensions return 0 when display is none.
> 
> Cheers,
> 
> -- 
> Eric Lemoine
> 
> Camptocamp France SAS
> Savoie Technolac, BP 352
> 73377 Le Bourget du Lac, Cedex
> 
> Tel : 00 33 4 79 44 44 96
> Mail : eric.lemoine at camptocamp.com
> http://www.camptocamp.com
> _______________________________________________
> Dev mailing list
> Dev at openlayers.org
> http://openlayers.org/mailman/listinfo/dev



-- 
Andreas Hocevar
OpenGeo - http://opengeo.org/
Expert service straight from the developers.