1055155 - [HiDPI] canvas dimensions incorrectly set in Nightly (ok in Aurora), in mapping webapp on pocz.org

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect)

Description

[Adrian Custer]

As of recent (2014-08-17) nightlies, canvas width and height recalculation seems broken.

The following webpage:
    http://pocz.org/ac/files/temp/webapp-canvas-size-bug/app.html
renders correctly in Aurora
  Built from https://hg.mozilla.org/releases/mozilla-aurora/rev/62a14ece9ea8
but incorrectly in Nightly
  Built from https://hg.mozilla.org/mozilla-central/rev/053a96e40244
and probably started failing a few revisions earlier; it took me a couple of days to figure out this was a bug in Firefox itself. Note that in Nightly, the images plotted on the canvas are stretched vertically. 

That there is some error occurring in firefox can be seen by opening the page in Nightly, then opening the developer tools, then going to the Inspector tool and the box model view. When the part of the page:
  html > body > section#core > canvas
is selected, the width='..' attribute in the inspector page code will be different from the width reported in the box model representation but both heights will be the same. In Aurora, the same setup will show that in the box model both values are different (about half) of the values as width='..' and height='...' of the canvas element. Somewhere internally, firefox is failing to track the height value correctly.

My app's code is making two successive calls to this code
    this._canvas.width  = width;
    this._canvas.height = height;
first setting the size of both elements to 0 and the second time setting them to values that are the parent DOM element size * window.devicePixelRatio (which is 2.22...) on this mac. The canvas itself is in a flex environment so the DOM size will differ from the internal canvas surface size.

A few days ago, I was intermittently getting an error that some file called 'script.js' in what seem like the deep internals of Firefox to the effect that something was being ignored because the 'this' context in that code was incorrect. However, I can not reproduce the error in more recent nightlies.

Hope that helps,
  ~adrian

Comment 1

[Alice0775 White]

I can reproduce the problem in Nightly34.0a1, and also Firefox24esr and IE11 on windows7.
I cannot reproduce the problem in   only   Google Chrome....

Comment 2

[Alice0775 White]

(In reply to Alice0775 White from comment #1)
> I can reproduce the problem in Nightly34.0a1, and also Firefox24esr and IE11
> on windows7.
> I cannot reproduce the problem in   only   Google Chrome....

Oops, please ignore the comment.

Comment 3

[Loic]

[Tracking Requested - why for this release]:

Regression range:
good=2014-07-22
bad=2014-07-23
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=63f44b4968c2&tochange=82df3654cd80

Comment 4

[Alice0775 White]

I can only reproduce on HiDPI monitor

Comment 5

[Alice0775 White]

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c38e0a84b54
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140722081925
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b489ff052163
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140722082830
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5c38e0a84b54&tochange=b489ff052163

In local build
Last Good: fc15aa692206
First Bad: 24c2f03786ac
Regressed by:
	24c2f03786ac	Daniel Holbert — Bug 984711 part 5: Add back handling for min-height:auto to nsFlexContainerFrame. r=mats This conceptually reverts changeset 7a289f49170f from bug 848539, though the code being modified looks a bit different now.

Comment 6

[Daniel Holbert [:dholbert]]

Bug 984711 (and its followup, but 984711) were for a spec-change, which is known & intended to change the sizing of flex items in some cases.  So, this is probably an expected change.

I'm marking this as blocking bug 1043520, which tracks breakage from that change. As noted there, breakage shouldn't be common, but the solution is probably for the site to be updated to interact nicely with "min-width:auto" on flex items.

I don't have a HiDPI monitor, so I can't directly test this at the moment. I suspect the issue is just that the min-content size of an element is larger there for some reason, and that's influencing the page now. (but it didn't before we added back min-width:auto support)

This can likely be worked around on the page by adding either "min-width:0" or "overflow:hidden" to the direct child of a flex item (whichever child is too large).  It's possible that there's a Firefox bug involved too, but I'm not sure about that yet.

Comment 7

[Alice0775 White]

(In reply to Daniel Holbert [:dholbert] from comment #6)
> I don't have a HiDPI monitor, so I can't directly test this at the moment.

You can emulate the problem if you set layout.css.devPixelsPerPx = 2 .

Comment 8

[Daniel Holbert [:dholbert]]

Thanks! With that, I can confirm that adding this style to the page fixes the issue:
#plot {
  min-height: 0;
}

The #plot element is a flex container (i.e. it has display:flex), but more importantly in this context, it's a flex item. (i.e. its parent has "display:flex")

So, it's prevented from being smaller than its auto-height, which is the desired height of its child, which in this case is set by the author to the following (from comment 0):
> parent DOM element size * window.devicePixelRatio (which is 2.22...) on this mac.

Given that there's a chain of DOM elements that have "height:100%" going up to the <html> element, the parent DOM element's size is the window size. So, the child ends up wanting 2.22 * the window height.  (For me, this is exactly 2x the window height, since I'm using the pref-tweak from comment 7.)

To allow #plot's flex container to size it below that intrinsic height (so that it doesn't overflow), we need to give it "min-height: 0".

Comment 9

[Daniel Holbert [:dholbert]]

Adrian, can you confirm that the style suggested in comment 8 fixes the bug for you?

Comment 10

[Adrian Custer]

Hey All,

Thanks for the diagnosis and the explanation. 

You may be right that what is required is fixing the CSS rules in the page, and that this is not a Mozilla bug. Unfortunately, 'min-height: 0;' is not the right tweak. 

Setting the 'min-height: 0;' as suggested in comment 8, does indeed reset the canvas height but not to the size available in the flexbox, but rather to the size given by the 100% height rules. The result is that the <canvas> element takes 100% of the height of the body rather than 100% of the size of the flexbox available to it and the footer is not visible on the page but requires a scroll to see---not the intended design and not what is seen in the Aurora builds.


So I have been trying to find a correct set of CSS rules under the new design but I am now getting more confused. This, on top of my lack of understanding of CSS, your recent changes, and the details of how the canvas element gets its height makes me want to fix this correctly once and for all by really understanding things.

It sounds like the flexbox rules, the 100% settings, and the <canvas> auto-magical CSS height assignment (and possibly the presence of floating divs) need to be rethought. The core complexity comes from the <canvas> element whose height is set by default to one value (the internal canvas draw surface size determined by the HTML attributes 'height' and 'width') but whose height can be set to another value when CSS 'takes over' (either if the CSS height and width are set by rule explicitly or when 'flexbox' takes over). So I want flexbox to take over but I am not sure how to trigger that.

The page would ideally have the structure:
  body <-- 100% of visible height
   header <-- fixed 20px height
   core   <-- flex 1, takes all
   footer <-- 12px fixed height
then plot and its internal canvas would simply fill the core (perhaps by being assigned 'height: 100%'. However, I would need some way to tell the canvas that it needs to go from using its default DOM size to using the DOM size available in the flexbox. It seems that the old style rules allowed me to do that since I got what I wanted in Aurora. So how do I do this under the new rules?

I've tried playing with 'overflow: hidden' but I can only replicate the behaviour of using 'min-height: 0'. Also, is there a writeup somewhere of why setting the 'min-height' shifts the behaviour of the <canvas> into its new logic? Finally, is there any good documentation on the circumstances which trigger <canvas> away from its default behaviour into following CSS size rules? 

Thanks for any clarification on this issue,
  ~adrian

Comment 11

[Daniel Holbert [:dholbert]]

'min-height: 0;' is at least part of the solution. I do see what you mean about the footer being pushed off -- I'll see if I can figure out why that's happening...

Comment 12

[Daniel Holbert [:dholbert]]

Looks like #core needs "min-height: 0" to fix that bottom part.

But really, you want to remove "height: 100%" from #core (since per comment 10, you don't actually want it to have 100% height -- you want it to be flexible), and replace that with:

  min-height: 0;  /* prevent min-content-based min-sizing */
  flex: 1;

With that, combined with "min-height: 0" on #plot, Nightly matches Aurora's rendering.

Comment 13

[Daniel Holbert [:dholbert]]

So, to answer your questions (without digging too much further into your design, because I don't have cycles to do that at the moment):

(In reply to Adrian Custer from comment #10)
> So how do I do this under
> the new rules?

See comment 12.

> I've tried playing with 'overflow: hidden' but I can only replicate the
> behaviour of using 'min-height: 0'.

Yeah, you don't want "overflow" here, since you're not actually wanting things to overflow (or be scrollable / clipped). It does disable the "min-height:auto" behavior (like "min-height:0" does), but if you don't actually want scrolling/clipping when space is constrained, then "min-height:0" is simpler.

> Also, is there a writeup somewhere of
> why setting the 'min-height' shifts the behaviour of the <canvas> into its
> new logic?

Not really. :) This is very new. The best documentation of this behavior is here in the flexbox spec:
 http://dev.w3.org/csswg/css-flexbox-1/#min-size-auto

(The only thing special about canvas is that it's an element with an intrinsic size & intrinsic ratio that you get to control. See below.)

> Finally, is there any good documentation on the circumstances
> which trigger <canvas> away from its default behaviour into following CSS
> size rules?

Basically, as I understand it:
 (A) The width and height *attributes* on a <canvas> specify the size of the canvas's bitmap, and also set its intrinsic dimensions and intrinsic ratio which are seen by CSS.

 (B) Ultimately, CSS is responsible for sizing the box that the canvas renders into on-screen. So, if CSS sizing properties are set to something non-default (e.g. "width", "height", "min-width", "flex", etc.), they'll generally override the canvas's intrinsic size.  The exact way that CSS gets a box-size in this case is complicated (since any of {min|max}-{width|height}, width, & height may be provided & need to be respected, and the canvas's intrinsic ratio also needs to be respected if constraints allow, and everything's a bit different if the canvas is a flex item).  The process in an inline context (non-flexbox) is largely documented here:
 http://www.w3.org/TR/CSS21/visudet.html#inline-replaced-width
...and the process in a flexbox context is documented in various parts of the flexbox spec (search for "aspect ratio"):
 http://dev.w3.org/csswg/css-flexbox-1/

Note that this chunk of the flexbox spec is under active development (e.g. the part relevant to this bug was actually rewritten again yesterday), so it may not quite match what's implemented currently, and it also may change some more.

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #13)
> Note that this chunk of the flexbox spec is under active development (e.g.
> the part relevant to this bug was actually rewritten again yesterday), so it
> may not quite match what's implemented currently, and it also may change
> some more.

(though presumably most/all remaining spec-changes will be just be tweaks to better handle edge cases, and shouldn't be very noticable to web content)

Comment 15

[Adrian Custer]

Daniel,

Thanks for taking the time to flesh this out as best you can; hopefully, it will help someone else along the way.

Closing the bug. With the two 'min-height: 0' (on #core and #plot) things behave as I would expect them to, and indeed 'flex: 1;' on #core what I wanted.

cheers.

Comment 16

[Daniel Holbert [:dholbert]]

Great, I'm glad I could help!

Also, since this was moved into component "Tech Evangelism" (i.e. "encourage/help web developer fix their site"), and you've fixed your site (from the sound of it), we can call this FIXED instead of INVALID. :)