831780 - scrollWidth, scrollHeight of DIV containing IMG reported as 40x40px until the image's actual size is known, regardless of CSS sizing of the IMG

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Daniel Trebbien]

Attachment: Test case

When an IMG element is first added to a DIV and the referenced image is not cached, the scrollWidth, scrollHeight of the parent DIV is calculated as 40x40px until the image's actual size is known, even if width/height attributes are specified on the IMG or the IMG is explicitly sized via CSS.

This is an issue first affecting Firefox 18.0.  Firefox 17.0.1 and Firefox ESR 17.0.2 are unaffected.

The presence of a DOCTYPE does not appear to matter.

The issue affects data URIs as well as images that are not in the browser cache.

Attached is a reproducible test case using various sizes of an image from Wikimedia Commons.  If you open the test case in Firefox 18.0 (Mac OS 10.7.5 and Windows 8 tested), the following is printed to the Web Console
Actual:
0: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16

Expected:
0: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16


I have also included code for downloading the images from Wikimedia Commons.  If you comment out the line setting the testImg src to a data URI and uncomment one of the lines setting the src to a Wikimedia Commons URL, you will see "0: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40" printed to the console until the image is cached.  Holding Shift while clicking the refresh button causes "0: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40" to always be printed.

Experimenting with Fiddler 2 to rate limit the download of the full-resolution image, I see that for the first several seconds, the DIV's scrollWidth and scrollHeight are 40px, but then the scrollWidth and scrollHeight are corrected:

[08:18:50.617] 0: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:51.612] 1: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:52.623] 2: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:53.615] 3: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:54.616] 4: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:55.617] 5: testDiv.scrollWidth = 40, testDiv.scrollHeight = 40,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:56.619] 6: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:18:57.621] 7: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
...
[08:20:08.623] 78: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
[08:20:09.617] 79: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16

Comment 1

[Daniel Trebbien]

Results for other browsers:

Safari 6.0.2:
0: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16

IE 8:
LOG: 0: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
LOG: 1: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16
LOG: 2: testDiv.scrollWidth = 16, testDiv.scrollHeight = 16,
testImg.offsetWidth = 16, testImg.offsetHeight = 16

Opera 12.12:
0: testDiv.scrollWidth = 16, testDiv.scrollHeight = 17,
testImg.offsetWidth = 16, testImg.offsetHeight = 16

Comment 2

[Daniel Trebbien]

Correction:  Mac OS 10.7.5 and Windows *7* tested

Comment 3

[Boris Zbarsky [:bzbarsky]]

Presumably the overflow area is different for some reason...

Comment 4

[Boris Zbarsky [:bzbarsky]]

That said, a current nightly build seems to show correct behavior.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, nevermind.  I see it now.

Comment 6

[Boris Zbarsky [:bzbarsky]]

This first appeared as a problem back in September, in this checkin range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=895f66c4eada&tochange=c09a0c022b2e

There was then a behavior change in November that makes us return 24x24 instead of 40x40 or 16x16.  That was in http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dd68409d7810&tochange=a761bfc192b5

The first range has bug 539356 as a plausible culprit.

Nothing in the second range jumps out at me, though it does have some scrollframe changes...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, here we go.

The change to 24x24 was due to bug 810876.

The initial change was in fact due to bug 539356.

The problem is that the broken-image placeholder can in fact be larger than the image.

Matt, roc, should we be including the altfeedback stuff in visual but not layout overflow, perhaps?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Simple testcase

Comment 9

[charles.kendrick]

Just a note on why this is a serious bug: this happens even if you provide sizes directly on the <img> tag (via HTML attributes or CSS), so we get a bad size from the browser even if we're explicitly telling the browser how big the image should be.

This in turn breaks the ability to determine the size of any content that includes images (inclusive of data URIs), which is a fundamental need of JavaScript frameworks that provide auto-sizing features.

We're really hoping for an 18.0.1 fix on this one, as the workarounds needed to get around this problem are really bad.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Actually, it's not quite clear to me why we're including the altfeedback in overflow at all.  It doesn't seem to be painted outside the image bounds...

Comment 11

[Matt Woodrow (:mattwoodrow)]

I think we want to only modify the visual overflow, yes.

It definitely was painting outside of the image bounds, and causing reftest failures.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #11)
> I think we want to only modify the visual overflow, yes.

I agree.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Only count the broken-image placeholder in the visual overflow, not the scrollable overflow.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Charles, I'm going to try to get this into 20 and 19, but the chance of us doing an 18.0.x for something that's not an actively exploited security vulnerability or a problem affecting lots and lots of users in extremely visible ways is pretty slim.  I appreciate that this isn't ideal from your point of view, but note that Firefox 19 will be shipping on February 19, while it's unlikely that something like this would get into an 18.0.x release before probably February 1 or so at this point.

For what it's worth, having bugs like this reported against our Aurora or even Beta builds, _before_ we release would make it very likely that we'd also fix them before release...  That's why we have the various testing channels, so people can test those builds.  I know it's not always possible to do that, but it really helps when it's done.

Alex, Lukas, if we do plan to do an 18.0.x for other reasons anyway, we might want to consider this for a ridealong.  This should be an _extremely_ safe change.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 703734 [details] [diff] [review]
Only count the broken-image placeholder in the visual overflow, not the scrollable overflow.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f08762b3f956

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 539356
User impact if declined: JS that tries to measure image sizes by measuring
   scrollable areas of elements containing those images will get wrong answers. 
   Might break some libraries/sites.
Testing completed (on m-c, etc.): Passes attached test.
Risk to taking this patch (and alternatives if risky): Very low risk, I believe.
   The alternative is to just let this ride the trains.
String or UUID changes made by this patch: None.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 703734 [details] [diff] [review]
Only count the broken-image placeholder in the visual overflow, not the scrollable overflow.

Approving for branches and will leave the release  branch nom so this is on the radar in case of another 18 respin (though at present one doesn't not appear likely).

Comment 17

[charles.kendrick]

Hi Boris, thanks for the comments re: release timing.  

At the moment I think this bug is probably causing a lot of issues on sites but few teams have figured out the underlying cause.  It's tricky to isolate since it goes away if images are cached.

I have asked our users to chime in if they know of prominent affected sites (not just ones based on our technology), which I think will help judge the urgency.

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/f08762b3f956

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a9c854f17c3

This is also in my queue to land on Aurora once it reopens.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/33542444da17

Comment 21

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 19 beta 3 - the scrollWidth and scrollHeight of the parent DIV are properly calculated when using the test cases attached in the Description and Comment 8.

Verified on Windows 7, Ubuntu 12.10 and Mac OS X 10.7.5:
Mozilla/5.0 (Windows NT 6.1; rv:19.0) Gecko/20100101 Firefox/19.0 Build ID: 20130123083802
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20100101 Firefox/19.0 Build ID: 20130123083802
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:19.0) Gecko/20100101 Firefox/19.0 Build ID: 20130123083802

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Batch edit: Bugs marked status-b2g18: affected after 2/13 branching of v1.0.1 are now also status-b2g18-v1.0.1: affected

Comment 23

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 20 beta 1 - scrollWidth and scrollHeight of the parent DIV are properly calculated

Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.8:
Build ID: 20130220104816
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0

Comment 24

[Ioana (away)]

Verified as fixed on Windows 7, Ubuntu 12.04 and Mac OSX 10.8.3, all 64bit, on Firefox 21 beta 6 (20130430204233).

scrollWidth and scrollHeight are properly computed when using the test case and simple test case attached to this bug.