594617 - [inc] When I increase browser width, svg-as-img doesn't grow wider until I reload

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Load URL (attachment 169879 [details])
    (this is just <img src="foo.svg"> with a 100%-wide <svg> root element)
 2. Make your browser window skinnier
     --> the SVG image gets smaller
 3. Make your browser window wider

EXPECTED RESULTS:
 The SVG image should get larger during step 3.

ACTUAL RESULTS:
 The SVG image remains the same size.  (But if I reload, it snaps up to a larger size.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: test 2: added border

If I add a border to the <img> element, it reveals that the <img> element itself isn't growing. (as shown in this modified testcase)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, ok.  The block containing the image doesn't know that it's actually using percentage sizing, so optimizes away the reflow of that line.

You need to make IsPercentageAware() in nsLineLayout.cpp return true in this case.

Also, shouldn't nsImageFrame override GetIntrinsicSize() and return mIntrinsicSize?  If that were happening, you'd just need to add it to the type whitelist there at the end of IsPercentageAware, I think.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #2)
> Also, shouldn't nsImageFrame override GetIntrinsicSize() and return
> mIntrinsicSize?

Aha -- yes! That fixes it.

> If that were happening, you'd just need to add it to the type
> whitelist there at the end of IsPercentageAware, I think.

Turns out this part isn't actually required. I stepped in and verified that IsPercentageAware() is returning PR_FALSE in this case -- but we still get correct behavior.  Is that wrong/bad?

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #3)
> > If that were happening, you'd just need to add it to the type
> > whitelist there at the end of IsPercentageAware, I think.
> 
> Turns out this part isn't actually required.

Ah, I think I found a testcase where the IsPercentageAware fix is required.  Patch + tests coming up.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: fix

This patch just applies both fixes that bz suggested in comment 2.  It includes a reftest that dynamically tweaks the width of a wrapper-div, and makes sure we end up resizing our SVG image.

The reftest fails without the IsPercentageAware()-tweak that I wasn't sure about in comment 3.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: fix for landing

Here's the fix for landing (with checkin comment included).

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/69a04435a68b

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

img-dyn-1.html is failing on Win2003, for example:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284500096.1284503532.7477.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284499736.1284503818.9168.gz

Made it random-if:
http://hg.mozilla.org/mozilla-central/rev/0eaaf91a5f3d

I'll leave it up to dholbert as to whether to reopen this bug or create a new bug to fix this failure.

Comment 9

[Daniel Holbert [:dholbert]]

The reftest failure has the div resized, but the SVG not resized yet.

I think it's a reftest-snapshot race condition, because it passed in the next Win2003 build after the logs cpearce mentioned:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1284502022.1284506062.20296.gz
and I can't reproduce it locally. (I tried a Win2003 opt build downloaded from Tinderbox, on a WinXP machine, and the reftest loads correctly no matter how many times I reload it)

I'm not immediately sure how this (a reftest-snapshot race condition) could happen here, though -- I'd assumed that we'd be reflowing the <img> synchronously with its div -- so we should never paint a larger div without also painting the larger <img>.  Is that not correct?