The font-size value, specified in a media query, is not updated when resizing an iFrame in MS Edge #52

chooie · 2018-03-30T05:50:09Z

Edit: Solved. Need to force a reflow after resizing the frame. See this comment. Quixote needs to be updated to do this automatically; see this comment.

Steps to Reproduce

You should just be able to checkout the repo and run ./tasks test:quick to see the issue in action

The failing test: https://github.com/chooie/incremental_it/blob/9a6b3220bc80f3cc7bac970312e8b1bfb5e1c454/src/application/client/content/_main_test.js#L86

A Workaround

https://github.com/chooie/incremental_it/blob/073279ebac9162f8db90373c58f69410b44148d7/src/application/client/content/_main_test.js#L86

The text was updated successfully, but these errors were encountered:

jamesshore · 2018-03-30T16:52:59Z

Thanks for the clear report, @chooie. I won't be able to work on this right away, so here's a few questions that will help me get a solution to you faster:

If you run the failing test on its own and visually inspect the results, is the font size changed? (Also, does the test still fail when it's run on its own?)
Does this occur on other browsers, or just MS Edge?
What happens if you add a timeout to the test, like this?

   it("is even bigger on wider devices", function(done) {
      cssHelper.frame.resize(cssHelper.mediumDeviceWidth, 500);
      setTimeout(function() {
        console.log("new", cssHelper.fontSize(element));
        assert.equal(cssHelper.fontSize(element), "64px");
        done();
      }, 1000);
  });

Thanks!

chooie · 2018-03-30T17:08:44Z

Hi James, I can try out your suggestions in a few hours. For now, though, I can confirm:

It only occurs on MS Edge ( Edge 16.16299.0 (Windows 10.0.0))
https://github.com/chooie/incremental_it/blob/9a6b3220bc80f3cc7bac970312e8b1bfb5e1c454/src/build/config/tested_browsers.js

chooie · 2018-03-30T23:07:20Z

The font size IS changed

>>>> ./tasks.sh test:quick
For more forgiving test settings, use 'loose=true'
Checking Node.js version: .
Linting JavaScript: ..
Testing CSS:
LOG: 'new', '64px'
.
Mobile Safari 11.0.0 (iOS 11.2.0): Executed 1 of 19 SUCCESS (0.378 secs / 0.013 secs)
Firefox 59.0.0 (Mac OS X 10.13.0) LOG: 'new', '64px'
.
Firefox 59.0.0 (Mac OS X 10.13.0): Executed 1 of 19 SUCCESS (0.17 secs / 0.008 secs)
Safari 11.0.3 (Mac OS X 10.13.3) LOG: 'new', '64px'
.
Safari 11.0.3 (Mac OS X 10.13.3): Executed 1 of 19 SUCCESS (0.728 secs / 0.031 secs)
Chrome Mobile 55.0.2883 (Android 7.1.1) LOG: 'new', '64px'
.
Chrome Mobile 55.0.2883 (Android 7.1.1): Executed 1 of 19 SUCCESS (1.154 secs / 0.032 secs)
Chrome 65.0.3325 (Mac OS X 10.13.3) LOG: 'new', '64px'
.
Chrome 65.0.3325 (Mac OS X 10.13.3): Executed 1 of 19 SUCCESS (0.255 secs / 0.002 secs)
Edge 16.16299.0 (Windows 10.0.0) LOG: 'new', '32px'
Edge 16.16299.0 (Windows 10.0.0) CSS: Layout Header is even bigger on wider devices FAILED
	AssertionError: expected '64px', but got '32px'
	   at fail (base/src/application/node_modules/vendor/proclaim-2.0.0.js?134aa623677f1257f43553dc0e41e969cf8ab7b4:308:9)
	   at proclaim.strictEqual (base/src/application/node_modules/vendor/proclaim-2.0.0.js?134aa623677f1257f43553dc0e41e969cf8ab7b4:39:13)
	   at exports.equal (base/src/application/node_modules/_assert.js?602de502ab1173dbd3a908ae09566397a3bace8d:62:5)
	   at Anonymous function (base/src/application/client/content/_main_test.js?c5565b94a936da60620c4f337886a8de916d2ddc:89:7)
Edge 16.16299.0 (Windows 10.0.0): Executed 1 of 19 (1 FAILED) ERROR (0.792 secs / 0.003 secs)
jake aborted.
Error: Karma tests failed
    at api.fail (/Users/chooie/code/projects/incremental_it/node_modules/jake/lib/api.js:336:18)
    at /Users/chooie/code/projects/incremental_it/node_modules/simplebuild-karma/src/index.js:64:26
(See full trace by running task with --trace)

3)If I add a timeout, it doesn't resolve the issue:

h1.New code

  it.only("is even bigger on wider devices", function(done) {
      this.timeout(10000);
      cssHelper.frame.resize(cssHelper.mediumDeviceWidth, 500);
      setTimeout(function() {
        console.log("new", cssHelper.fontSize(element));
        assert.equal(cssHelper.fontSize(element), "64px");
        done();
      }, 1000);
    });

h1.Results

>>>> ./tasks.sh test:quick
For more forgiving test settings, use 'loose=true'
Checking Node.js version: .
Linting JavaScript: .
Testing CSS:
Mobile Safari 11.0.0 (iOS 11.2.0) LOG: 'new', '64px'
.
Mobile Safari 11.0.0 (iOS 11.2.0): Executed 1 of 19 SUCCESS (1.329 secs / 1.002 secs)
Firefox 59.0.0 (Mac OS X 10.13.0) LOG: 'new', '64px'
.
Firefox 59.0.0 (Mac OS X 10.13.0): Executed 1 of 19 SUCCESS (1.482 secs / 1.008 secs)
Chrome Mobile 55.0.2883 (Android 7.1.1) LOG: 'new', '64px'
.
Chrome Mobile 55.0.2883 (Android 7.1.1): Executed 1 of 19 SUCCESS (1.751 secs / 1.006 secs)
Safari 11.0.3 (Mac OS X 10.13.3) LOG: 'new', '64px'
.
Safari 11.0.3 (Mac OS X 10.13.3): Executed 1 of 19 SUCCESS (1.806 secs / 1.002 secs)
Chrome 65.0.3325 (Mac OS X 10.13.3) LOG: 'new', '64px'
.
Chrome 65.0.3325 (Mac OS X 10.13.3): Executed 1 of 19 SUCCESS (1.566 secs / 1.005 secs)
Edge 16.16299.0 (Windows 10.0.0) LOG: 'new', '32px'
Edge 16.16299.0 (Windows 10.0.0) CSS: Layout Header is even bigger on wider devices FAILED
	Error: AssertionError: expected '64px', but got '32px' (http://10.0.2.2:9876src/application/node_modules/vendor/proclaim-2.0.0.js:308)
Edge 16.16299.0 (Windows 10.0.0): Executed 1 of 19 (1 FAILED) ERROR (8.842 secs / NaN secs)
jake aborted.
Error: Karma tests failed
    at api.fail (/Users/chooie/code/projects/incremental_it/node_modules/jake/lib/api.js:336:18)
    at /Users/chooie/code/projects/incremental_it/node_modules/simplebuild-karma/src/index.js:64:26
(See full trace by running task with --trace)

jamesshore · 2018-03-30T23:11:41Z

Thanks for checking. Unfortunately, these are the worst ones to troubleshoot. It seems likely that there's a bug in MS Edge, because the CSS API we're using is very straightforward. :-b It may be related to the font caching gotcha mentioned in the readme. I'll poke around and see what I can discover. It may be a while before I can look at it, though.

woldie · 2018-04-02T06:39:35Z

Font size is a tricky one. It does appear the font size is increased on your 'Hello' there, and your inspector seems to confirm. I am suspicious that what you're seeing is similar to a bug from last year in Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/9748455/ I wonder how much you can strip your CSS and HTML down and have it still reproduce for you. It would be pretty cool to see the Edge developers using Quixote to test their CSS engine, maybe you could make a bug report and use Quixote in the code sample! :) Can you get the assertion to pass in Edge if you float all your media queries up to the tippity top of the .css file? Perhaps there is an naive ordering mishap going on in the getComputedStyle() implementation on Edge?

…

On Fri, Mar 30, 2018 at 4:11 PM, James Shore ***@***.***> wrote: Thanks for checking. Unfortunately, these are the worst ones to troubleshoot. It seems likely that there's a bug in MS Edge, because the CSS API we're using is very straightforward. :-b It may be related to the font caching gotcha mentioned in the readme. I'll poke around and see what I can discover. It may be a while before I can look at it, though. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJxoYKzC9i_7z1ktPX6K0KmWtHfLGXvQks5tjruugaJpZM4TBU-b> .

jamesshore · 2018-04-02T21:20:49Z

I agree with @woldie's suggestion of a simplest-possible test case. We should be able to shrink the CSS down to not much more than the font-size and media declarations, and even simplify the assertion for a MS Edge bug report. Also, if at any point the code starts passing, that will be very informative for us in terms of documenting the issue and coming up with a work-around that's compatible with frame.resize().

@chooie's current code has this assertion:

 assert.equal(cssHelper.fontSize(element), "64px");

That can be changed to this for the purposes of a bug report:

var domElement = element.toDomElement();
var domFrame = cssHelper.frame.toDomElement();

var styles = domFrame.contentWindow.getComputedStyle(domElement);
var fontSize = styles.getPropertyValue("font-size");
assert.equal(fontSize, "64px");

It's possible we're doing something incorrect with the way we're setting up the frame, too--there's a lot of fiddly edge cases associated with that code. The best reproduction code would get rid of Quixote entirely, I think, and just do the frame setup and resize directly in the test.

jamesshore · 2018-04-02T21:48:04Z

This gist could be a helpful starting point for making a repro test case. It has the code for defining a frame already inlined: https://gist.github.com/jamesshore/c09d071eb5b16b7645f2

The resize code is very simple and should also be easy to inline:

this._domElement.setAttribute("width", "" + width);
this._domElement.setAttribute("height", "" + height);

chooie · 2018-04-02T21:55:55Z

Thanks, folks. I'll have a look at this tonight and work on getting a minimal example going.

chooie · 2018-04-03T04:45:11Z

I can now confirm that this is probably a bug in MS Edge.

Open this file in it and it will demonstrate the issue in the console:

ms_edge_iframe_bug_demo.html

<html>
  <head>
    <title>MS Edge Bug Example</title>
  </head>
  <body>
    <p>
      The header in the iFrame displays fine. `getPropertyValue("font-size")`,
      however, is incorrect
    </p>
<script>
function demonstrateFontSizeBug() {
  let iFrame = makeIframeWithStylesAndContent();
  let iFrameDocument = iFrame.contentWindow.document;
  let headerElement = iFrameDocument.getElementById("header");

  // Default works fine
  let headerStyles = iFrame.contentWindow.getComputedStyle(headerElement);
  let fontSize = headerStyles.getPropertyValue("font-size");
  console.log("Expect '10px' and got: '" + fontSize + "'");

  // MS Edge doesn't play nice with media queries, all other test browsers do
  iFrame.style.width = "1000px";
  iFrame.style.height = "500px";

  headerStyles = iFrame.contentWindow.getComputedStyle(headerElement);
  fontSize = headerStyles.getPropertyValue("font-size");
  console.log("Expect '64px' and got: '" + fontSize + "'");
}

function makeIframeWithStylesAndContent() {
  let iframe = document.createElement("iframe");
  iframe.id = "my-iframe";
  let css = `
<style type="text/css">
  .header {
      font-size: 10px;
  }

  /* Media queries must come after the 'default' styles */
  @media screen and (min-width: 640px) {
    .header {
        font-size: 64px;
    }
  }
</style>
`;

  let headerText = `
<h1 id="header" class="header">Hello</h1>
`;
  document.body.appendChild(iframe);

  let iframeDocument = iframe.contentWindow.document;
  iframeDocument.write(`
${css}
${headerText}
`);

  return iframe;
}

demonstrateFontSizeBug();
</script>
  </body>
</html>

Thank you for your help (and Quixote for finding this issue!). I'll file a bug report with the Edge devs.

chooie · 2018-04-03T05:09:53Z

Issue: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/16714064/

chooie · 2018-04-03T05:31:41Z

Update: Putting the call to getComputedStyle in a setTimeout gives the correct value:

setTimeout(function() {
    headerStyles = iFrame.contentWindow.getComputedStyle(headerElement);
    fontSize = headerStyles.getPropertyValue("font-size");
    console.log("Expect '64px' and got: '" + fontSize + "'");
}, 0);

Would a workaround like this work for Quixote?

woldie · 2018-04-03T06:14:11Z

Nice work filing that bug. Interesting that setTimeout allows getComputedStyle() to "catch up" with the view. Someone at MSFT forgot to set an internal reflow dirty flag or some such that the parent window of an iframe can observe. :) It might be some remnant of my beloved IE6 in there doing its best, but still falling short. (I miss you IE!) Anyhow, my vote would be to not wrap getComputedStyle() in a timeout to get the right answer in this Edge case (I ... I couldn't stop my hands, send the pun police to arrest me!) Part of the implicit contract of the DOM and getComputedStyle() is that it should block until reflow completes in order that the computed styles returned match what is displayed. The browser just isn't behaving to spec in this case and needs its belts and hoses checked.

jamesshore · 2018-04-03T17:58:31Z

Interesting that setTimeout works for your test case but not when you tried it in this comment.

I'm going to leave this issue open for now as something to investigate further. At the very least, Quixote can provide a browser detect for this issue.

chooie · 2018-04-05T05:40:53Z

Hmm, I tried it out again with the original code and it's working now (with the timeout 'hack'). Either I didn't save the file or it was an extraneous mistake. For now, this workaround works for me! I'll follow up if something better turns up.

chooie · 2018-04-05T06:40:02Z

Looks like it's a flaky test from the race condition introduced by Edge. For integration tests, the problem is particularly prevalent (just on Edge) if the timeout is set to 0.

jamesshore · 2018-04-06T22:12:49Z

Sometimes it's helpful to force a browser reflow. This will do it:

domElement.offsetHeight;

Does that help in this case?

chooie · 2018-04-10T01:34:04Z

It works and brings back synchronicity! Thanks, James. This is the best workaround so far.

function forceBrowserReflow() {
  document.body.offsetHeight;
}

jamesshore · 2018-04-10T01:38:00Z

Glad to hear it! I encountered a similar problem when adding a class to an element. I don't remember which browser was affected, but I had to force a reflow before the class took effect. It makes sense that resizing the frame could have the same kind of issue.

I think the best fix for this would be to modify frame.resize to force reflow after resizing. That should solve this problem universally. I'll leave this issue open until then.

chooie · 2018-04-10T02:04:50Z

Also, a quick correction:

function forceBrowserReflow(element) {
  element.offsetHeight;
}

I was experiencing another async issue with Firefox 59.0.0 (Mac OS X 10.13.0) when getting the font-size. Doing the reflow on the element under test, instead of the document, gets around this issue.

jamesshore · 2020-04-09T23:02:56Z

I've successfully reproduced this issue with the following code in _q_frame_test.js (in the describe("instance") block, using the 0.14.1 version of the code):

		it("Issue #52", function() {
			frame.add(
				"<style type='text/css'>" +
					".header { font-size: 20px; }" +
					"@media (min-width: 640px) {" +
						".header { font-size: 40px; }" +
					"}" +
				"</style>"
			);
			var element = frame.add("<h1 class=header>hello</h1>");
			frame.resize(200, 500);
			assert.equal(element.getRawStyle("font-size"), "20px");

			frame.resize(800, 500);
			// var forceReflow = element.toDomElement().offsetHeight;
			assert.equal(element.getRawStyle("font-size"), "40px");
		});

As written, this test fails in these browsers:

IE 11.0.0 (Windows 7.0.0)
Edge 18.18362.0 (Windows 10.0.0)

It passes in these browsers:

Safari 13.1.0 (Mac OS X 10.14.6)
Firefox 75.0.0 (Mac OS X 10.14.0)
Mobile Safari 13.0.4 (iOS 13.3.0)
Chrome 80.0.3987 (Mac OS X 10.14.6)
Chrome Mobile 74.0.3729 (Android 0.0.0)

Uncommenting the forceReflow line makes it pass in all of these browsers. IE 8 wasn't tested; it can't handle the <style> tag.

jamesshore · 2020-04-10T00:03:14Z

A similar issue is present when using viewport-relative sizes. This one exists in all browsers tested except Firefox 75. (IE 8 doesn't support viewport-relative sizes.) The same fix works.

* dev: Update distribution files Tweak changelog Document reflow fix for 0.14.2 release Force reflow after resizing frame (fixes issue #52)

jamesshore · 2020-04-10T00:23:09Z

Fixed in v0.14.2.

jamesshore added the bug label May 10, 2019

jamesshore added ready to integrate in progress labels Apr 9, 2020

jamesshore added a commit that referenced this issue Apr 10, 2020

Force reflow after resizing frame (fixes issue #52)

1922f4f

jamesshore added done and removed in progress ready to integrate labels Apr 10, 2020

jamesshore added a commit that referenced this issue Apr 10, 2020

INTEGRATE: Force reflow after resizing frame

76e2b77

* dev: Update distribution files Tweak changelog Document reflow fix for 0.14.2 release Force reflow after resizing frame (fixes issue #52)

jamesshore closed this as completed Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The font-size value, specified in a media query, is not updated when resizing an iFrame in MS Edge #52

The font-size value, specified in a media query, is not updated when resizing an iFrame in MS Edge #52

chooie commented Mar 30, 2018 •

edited by jamesshore

jamesshore commented Mar 30, 2018 •

edited

chooie commented Mar 30, 2018

chooie commented Mar 30, 2018 •

edited

jamesshore commented Mar 30, 2018

woldie commented Apr 2, 2018 via email

jamesshore commented Apr 2, 2018

jamesshore commented Apr 2, 2018 •

edited

chooie commented Apr 2, 2018

chooie commented Apr 3, 2018

chooie commented Apr 3, 2018

chooie commented Apr 3, 2018

woldie commented Apr 3, 2018 via email •

edited by jamesshore

jamesshore commented Apr 3, 2018

chooie commented Apr 5, 2018

chooie commented Apr 5, 2018

jamesshore commented Apr 6, 2018 •

edited

chooie commented Apr 10, 2018

jamesshore commented Apr 10, 2018

chooie commented Apr 10, 2018

jamesshore commented Apr 9, 2020 •

edited

jamesshore commented Apr 10, 2020

jamesshore commented Apr 10, 2020

The font-size value, specified in a media query, is not updated when resizing an iFrame in MS Edge #52

The font-size value, specified in a media query, is not updated when resizing an iFrame in MS Edge #52

Comments

chooie commented Mar 30, 2018 • edited by jamesshore

Steps to Reproduce

A Workaround

jamesshore commented Mar 30, 2018 • edited

chooie commented Mar 30, 2018

chooie commented Mar 30, 2018 • edited

jamesshore commented Mar 30, 2018

woldie commented Apr 2, 2018 via email

jamesshore commented Apr 2, 2018

jamesshore commented Apr 2, 2018 • edited

chooie commented Apr 2, 2018

chooie commented Apr 3, 2018

chooie commented Apr 3, 2018

chooie commented Apr 3, 2018

woldie commented Apr 3, 2018 via email • edited by jamesshore

jamesshore commented Apr 3, 2018

chooie commented Apr 5, 2018

chooie commented Apr 5, 2018

jamesshore commented Apr 6, 2018 • edited

chooie commented Apr 10, 2018

jamesshore commented Apr 10, 2018

chooie commented Apr 10, 2018

jamesshore commented Apr 9, 2020 • edited

jamesshore commented Apr 10, 2020

jamesshore commented Apr 10, 2020

chooie commented Mar 30, 2018 •

edited by jamesshore

jamesshore commented Mar 30, 2018 •

edited

chooie commented Mar 30, 2018 •

edited

jamesshore commented Apr 2, 2018 •

edited

woldie commented Apr 3, 2018 via email •

edited by jamesshore

jamesshore commented Apr 6, 2018 •

edited

jamesshore commented Apr 9, 2020 •

edited