Resize iframe logic should account for padding

Closes canjs/3858

Author: m-mujica

demo_frame.js

@@ -132,27 +132,39 @@ module.exports = function(node) {
 		txt = txt.replace(/</g, "&lt;");
 		return typeof prettyPrintOne !== "undefined" ? prettyPrintOne(txt) : txt;
 	}
-
+	// Given the content height and the compute styles of the element,
+	// compute the total height of the box
+	function getTotalHeight(height, computed) {
+		return height +
+			parseInt(computed.marginTop, 10) +
+			parseInt(computed.marginBottom, 10) +
+			parseInt(computed.borderTopWidth, 10) +
+			parseInt(computed.borderBottomWidth, 10) +
+			parseInt(computed.paddingTop, 10) +
+			parseInt(computed.paddingBottom, 10);
+	}
 	function resizeIframe() {
 		var frame = node.getElementsByTagName("iframe")[0];
-		var height = frame.contentWindow.document.body.scrollHeight;
-
-		var tolerance = 5; // pixels
-		var low = height - tolerance;
-		var high = height + tolerance;
+		var computed = getComputedStyle(frame);
 
-		// turns "150px" to 150, and "" to 0
-		var getCssHeight = function() {
-			var h = frame.style.height;
-			return Number(h.substr(0, h.length - 2) || 0);
-		};
+		if (!frame.contentDocument || !frame.contentDocument.body) {
+			return;
+		}
 
-		var cssHeight = getCssHeight();
+		// compute height tolerance range
+		var tolerance = 5; // pixels
+		var desiredHeight = getTotalHeight(
+			frame.contentDocument.body.scrollHeight,
+			computed
+		);
+		var low = desiredHeight - tolerance;
+		var high = desiredHeight + tolerance;
 
 		// Setting the height causes the next resizeIframe call to get a different
-		// height reading (lower); The range/tolerance logic is added to prevent the
-		// continous shrinking of the iframe
-		if (cssHeight < low || cssHeight > high) {
+		// height reading (lower); The range/tolerance logic is added to prevent
+		// the continous shrinking of the iframe
+		var currentHeight = parseInt(computed.height, 10);
+		if (currentHeight < low || currentHeight > high) {
 			iframe.style.height = Math.min(high, 600) + "px";
 		}
 

test.js

@@ -388,6 +388,34 @@ describe("bit-docs-tag-demo", function() {
 			});
 		});
 
+		describe("resize iframe logic adds up border/padding/margin", function() {
+			before(function() {
+				return ctx.browser
+					.newPage()
+					.then(function(p) {
+						ctx.page = p;
+						return ctx.page.goto("http://127.0.0.1:8081/test/temp/heightBoxModel.html");
+					})
+					.then(function() {
+						return ctx.page.waitForFunction(function() {
+							var iframe = document.querySelector("iframe");
+							return iframe && iframe.style.height === "600px";
+						});
+					});
+			});
+
+			it("resizes height up to 600px", function() {
+				ctx.page
+					.evaluate(function() {
+						return document.querySelector("iframe").style.height;
+					})
+					.then(function(height) {
+						assert.equal(height, "600px");
+					});
+			});
+		});
+
+
 		describe("multiple instances", function() {
 			this.timeout(8000);
 
@@ -420,8 +448,8 @@ describe("bit-docs-tag-demo", function() {
 						};
 					})
 					.then(function(r) {
-						assert.equal(r.wrappers, 5, "four wrappers exists");
-						assert.equal(r.injected, 5, "four injected into wrappers");
+						assert.equal(r.wrappers, 6, "four wrappers exists");
+						assert.equal(r.injected, 6, "four injected into wrappers");
 					});
 			});
 		});

test/demos/demo-height-box-model.html

@@ -0,0 +1,12 @@
+<div id="demo-html"></div>
+<script id="demo-source">
+	for (var i = 0; i < 5; i += 1) {
+		var div = document.createElement("div");
+		div.style.height = "100px";
+		div.textContent = "100px";
+		document.body.appendChild(div);
+	}
+	var frame = window.frameElement;
+	frame.style.paddingTop = "100px";
+	frame.style.paddingBottom = "100px";
+</script>

test/generate.js

@@ -16,7 +16,8 @@ function allDemos() {
 		"demo-without-ids",
 		"demo-without-js",
 		"demo-complex",
-		"demo-height"
+		"demo-height",
+		"demo-height-box-model"
 	]
 		.map(function(demo) {
 			return "<h2>" + demo + "</h2>" + makeWrapper(demo);
@@ -45,6 +46,10 @@ var docMap = Promise.resolve({
 		name: "height",
 		body: makeWrapper("demo-height")
 	},
+	heightBoxModel: {
+		name: "heightBoxModel",
+		body: makeWrapper("demo-height-box-model")
+	},
 	index: {
 		name: "index",
 		body: allDemos()