534808 - Image resized by percentage height does not change width in response to indirect height change

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (dynamic)

Based on a piece of layout/reftests/bugs/364066-1.html (bug 364066).

Comment 1

[Jesse Ruderman]

Attachment: reference (static)

Comment 2

[Boris Zbarsky [:bzbarsky]]

The image changes width fine.  What doesn't change is the table cell.

The problem is that in this case a height change in fact leads to an intrinsic width change.  This looks like a regression from bug 502288.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Except of course we do clear ancestor intrinsics in this case...

Comment 4

[Boris Zbarsky [:bzbarsky]]

Er, no.  We in fact do not.  Doing that should fix this bug; testing.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Er, and I take that back once again.  For height we have:

    return NS_CombineHint(hint,
                          NS_SubtractHint(nsChangeHint_ReflowFrame,
                                          nsChangeHint_ClearDescendantIntrinsics));

Comment 6

[Boris Zbarsky [:bzbarsky]]

Aha!  The key is that the <div> that is a _child_ of the <div> we're changing the height on has an intrinsic width that depends on the height of the ancestor.

If we want to have that to be the case (and currently it is because GetPercentHeight in nsLayoutUtils calls itself on the containing block), then changes to height do in fact need to clear _descendant_ intrinsics in general.

David, does that sound ok?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes, that sounds fine.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Is this broken on 1.9.2 as well?  It seems like something we should get fixed there.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Is this broken on 1.9.2 as well?

Yes.  I'll try to get together a patch for this today; then we can land it on m-c and work on getting on branch.

If you think this should block, please nominate?  I'm being a little torn about that at this late date...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

At this point, I'd be ok with fixing it in a .1, although I don't seem to be able to nominate it for that.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: m-c fix

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: 1.9.2 merge

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 419343 [details] [diff] [review]
m-c fix

r=dbaron

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 419344 [details] [diff] [review]
1.9.2 merge

r=dbaron

Comment 15

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/89d33cf3490c

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is a layout regression caused by slight over-optimization.  The fix is very low risk, and I think we should really get it in before shipping a major update to 3.6.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 419344 [details] [diff] [review]
1.9.2 merge

a=beltzner for 1.9.2.2, in a fantastic deal with dbaron by which way everyone wins!

Comment 18

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4a8ddcbeef48

Comment 19

[Aakash Desai [:aakashd]]

verified FIXED on build using the testcase provided:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2