Util.getRenderedSize does not calculate with inherited style

[gregers]

Util.getRenderedSize inserts contentHTML directly on document.body instead of where the content will be inserted. So if the resulting size might be different if for instance the popup content inherits text/font styling.

This has been discussed on the mailinglist:  http://n2.nabble.com/FramedCloud-and-autoSize-td1829440.html Matthew Atkins Jun 20, 2008; 04:34pm

I've created a patch for Util.js that adds an optional property to the options object for getRenderedSize. If this option is supplied, the default (document.body) will be overridden by this value, and the content will be inserted here instead of the body root.

I also changed Popup.js, so getRenderedSize uses map.layerContainerDiv instead of body.

[gregers]

Example to reproduce the bug

[tschaub]

This looks legit to me. I wonder if we can get uz (sr. popup) to weigh in/review.

[euzuro]

Changes: 1) removed noise 2) renamed 'container' to 'containerElement' to be explicit/clear 3) when used in popuplandia, don't always assume we'll have a this.map. there is a test in FramedCloud that caught this. ALL TESTS (more or less) PASS FF3, IE7

[euzuro]

4) make the patch not work anymore

....trying to figure out where I went wrong. back in a moment...

[euzuro]

forgot to rename the options.container -> options.containerElement

[euzuro]

ok. new patch works. all tests* pass in ff3/ie7.

...this is a nice upgrade for the popups. nice work on making a small, slick, unintrusive patch. And major kudos for a good example/acceptance test.

My only requests before green light:

1) CLA from 'gregers' 2) rename the html example included with this ticket to something more relevant to the bug, and include it in a final patch.

* overviewMap test is failing before and after application of patch in both ff3 and ie7. i'm assuming it's unrelated.

[gregers]

Corporate Contributor License Agreement faxed (FINN.no AS). I added all members of the team just in case they need it later.

[crschmidt]

closed by r8906.

[crschmidt]

Breaks in IE7.

[crschmidt]

In standards mode on IE7, this change causes the width of the container to not be computed correctly.

 http://www.gis-hosting.lu/ietestcase/ol28.html

vs.

 http://www.gis-hosting.lu/ietestcase/ol27.html

Since this is a regression, this is a blocker for the 2.8 release.

[gregers]

After hours of debugging (and swearing) I found out that IE7 wouldn't calculate the correct size when the element is absolutely positioned inside the containerElement. The absolute position along with style.left="-9999px" is used so the element is not visible on the screen. Since display="none" doesn't affect layout. This can also be accomplished (and more beautiful imho) with style.visibility="hidden". I'm not sure about browser compatibility though, and haven't had much time to test it. I ran the Util test-case in my installed browser (FF 3.0.10, Opera 9.63, IE8 with and w/o IE7 standards mode, Chrome 2.0.172.8), and the getRenderedDimensions test passed.

Is this an ok solution?

[gregers]

Patch to fix IE7 bug

[crschmidt]

I think this is a reasonably good solution. I've tested in IE6, IE7, Firefox 2, Firefox 3, Safari3 and seen no adverse affects from this in the example given by the reporter of the bug ( http://crschmidt.net/mapping/popup/ol28.html is my patched copy). I can't think of anything else to try to give this a more conclusive pass.

Actually, I came up with something: http://openlayers.org/dev/tests/manual/rendered-dimensions.html

This test fails with the patch applied: the width of the element is detected as being the width of the page, in both cases. (Essentially, what this does is test the getRenderedDimensions size with and without the application of a CSS element). I can't say why this is without further investigation, but it seems like it is probably something to consider fixing before applying this patch.

[gregers]

Ah, that's true. When the "<p>" doesn't have position:absolute, it will expand to it's container (the body). I also tried to use float:left instead of position:absolute, but it had the same effect.

The rendered-dimensions test didn't have a DOCTYPE declaration, so it was rendered in quirksmode. I tried all possible combinations of quirks and standards mode for both tests, with and without position:absolute. For quirksmode it's safe to set position:absolute, but for standards mode the two tests are opposite. rendered-dimensions require position:absolute, and the ietestcase can't have position:absolute. Still haven't figured out why or how to detect the difference... Any ideas? I have a slight suspicion that it might be related to the VML, since the bug suddenly disappeared when I removed the GML layer while debugging yesterday.

[gregers]

It's not related to VML, but maybe to the GML layer or SelectFeature. If I create and add a popup at the end of makeMap(), with the same content and properties as in the onSelect callback. The popup size is correct.

I've inspected the differences with the IE8 developer tools (in IE7 mode). But the the DIV generated by getRenderedSize for the working and non-working popup are equal, except for container.style.width. The width is set on the container by getRenderedSize from the content.scrollWidth. But for some reason the scrollWidth differ. They are siblings of the same container. Have the same classes/attributes/children/content...

Wait... I found out why. By adding a 5 second timer on the working popup - it stopped working. So something changes after makeMap(). Will have to look more at this tomorrow.

[gregers]

I have it working for IE (7 & 8), FF 3, Chrome, but it stopped working in Opera.

[gregers]

Opera doesn't like position:absolute when only size.h is specified. Avoiding position:absolute works for the ietestcase, but not for the rendered-dimensions test, since the <p> will expand to 100% of body. Haven't found a workaround for Opera.

[crschmidt]

I'm willing to stipulate that the test is bogus, if we are sufficiently convinced that it doesn't represent reality. I just fear that it closely enough represents reality -- for example, since we're putting these <div>s inside the viewport, that they will expand to fill the viewport. If that's not going to happen, then don't worry about how the test currently behaves: we should fix the test.

[gregers]

From IRC yesterday: Also, how important are these issues? - The width of the content is incorrectly calculated when IE is in quirks mode. So the popup get's scrollbars. - The "theme"-stylesheet is included by the Map.initialize, but it's not necessarily loaded when the first popup is loaded. Any help on this issue would be greatly appreciated. I don't have much more time to spare on this issue, so maybe it's better to postpone it to 2.9 if any of the issues mentioned are important?

17:41 < crschmidt> gregers: I think we can ignore any test in which the theme is not loaded yet being an issue 17:41 < crschmidt> we encourage people to load the stylesheets in a wya that this is not an issue 17:41 < crschmidt> so the fact that people don't follow that advice should not be considered a flaw in OL

[gregers]

Updated to work in Opera as well - small bugfix

[gregers]

The problem was that Opera and IE7 can't handle a node with position:aboslute if it "inherits" position:absolute from a parent.

By avoiding position:absolute if it's already inherited solved the problem for all browsers I have installed: FF 3.0.10, Opera 9.64, IE8 with and w/o IE7 standards mode, IE6 and Chrome 2.0.172.8.

[crschmidt]

Confirmed that this appears to work:

• In all existing demos, in FF, Safari, IE7.
• In the demonstrated failure case in IE7.
• Does not appear to fail any regression tests.
• Appears to be a generally sane solution.

With that, I'm committing this and marking for pullup.

[crschmidt]

(In [9384]) Committing an updated fix for #1906. This fixes a 2.7->2.8 regression in particular behavior with regard to determining the size of a popup. thanks to the absolutely tireless work of gregers on this issue! Also, in case anyone was wondering? Browsers suck. (Pullup #1906)

[crschmidt]

(In [9406]) Pullups for OL 2.8 RC3.

jQuery lib fix (Closes #1391) getRenderedSize regression (Closes #1906) element.scrolls error with panzoombar (Closes #2054) createUrlObject bug (Closes #2060) google layer in late rendered maps (Closes #2075) IE6/Lang.nb bug (Closes #2093) Layer.TMS/TileCache bugs (Closes #2099) (Closes #2100) Graphic names issues (Closes #2101)