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

[chooie]

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

[jamesshore]

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:

1) 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?) 2) Does this occur on other browsers, or just MS Edge? 3) 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]

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

2. 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]

1) 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]

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]

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 notifications@github.com 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 https://github.com/jamesshore/quixote/issues/52#issuecomment-377643317, or mute the thread https://github.com/notifications/unsubscribe-auth/AJxoYKzC9i_7z1ktPX6K0KmWtHfLGXvQks5tjruugaJpZM4TBU-b .

[jamesshore]

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]

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]

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

[chooie]

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]

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

[chooie]

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]

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]

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]

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]

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]

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

domElement.offsetHeight;

Does that help in this case?

[chooie]

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

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

[jamesshore]

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]

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]

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