290297 - Acid2 moves when hovering over nose

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

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

The Acid2 test page moves around when hovering over the nose.  According to
Hixie, the nose should turn blue, but nothing should move.  This makes sense. 
The style system shouldn't even be generating a reflow hint, as far as I can
tell, so we have at least one problem, if not two.

Comment 1

[Jo Hermans]

Hmmmmm .... In Firefox 1.0.2, it doesn't move and the node turns blue, as expected.

Comment 2

[Ben Fowler]

Attachment: testcase

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050413
Firefox/1.0+

I am seeing two lines of pixels corresponding to the <div>s inside smile
appear to move upwards.

I enclose a shorter file that shows the problem.

Comment 3

[Ben Fowler]

Konqueror does not have "moves around" problem, but does not display the page as
specified. 

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, we have at least two problems:

1)  We're getting a reflow hint, as David said.  This is due to the fix for bug
    275139 not being quite right enough to fix the wrongness of the code.  I've
    filed bug 290362 on this.
2)  Once we get a reflow hint, we're oscillating.  Leaving this bug to handle
    that (and will post a testcase that shows this problem even with bug 290362
    fixed).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Somewhat minimized testcase

This still shows the reliable oscillating behavior.  If I remove the clearance,
I get a one-time {inc} bug on hover and then we don't change the layout
anymore.

Comment 6

[Boris Zbarsky [:bzbarsky]]

roc, I know you love clearance... ;)

Comment 7

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

Attachment: fix

The problem is that we're not recovering the previous margin before we reflow a
dirty line, because the line is dirtied by PropagateFloatDamage which happens
after margin recovery. We should recover the margin *after*
PropagateFloatDamage so it gets recovered even if PropagateFloatDamage dirtied
the line. The comment currently in the code says this is unsafe because
PropagateFloatDamage depends on aState.mPrevCarriedOutMargin, but as far as I
can tell, in fact nothing under PropagateFloatDamage actually uses it.

Comment 8

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

Comment on attachment 181227 [details] [diff] [review]
fix

This fixed an incremental reflow bug which happens to manifest on the Acid2
test.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 181227 [details] [diff] [review]
fix

a=asa

Comment 10

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

checked in