Acid2 moves when hovering over nose

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
12 years ago
3 years ago

People

(Reporter: dbaron, Assigned: roc)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
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

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

Comment 2

12 years ago
Created attachment 180698 [details]
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

12 years ago
Konqueror does not have "moves around" problem, but does not display the page as
specified. 
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).
Depends on: 290362
Created attachment 180773 [details]
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.
roc, I know you love clearance... ;)

Updated

12 years ago
Keywords: testcase
Created attachment 181227 [details] [diff] [review]
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.
Assignee: dbaron → roc
Status: NEW → ASSIGNED
Attachment #181227 - Flags: superreview?(dbaron)
Attachment #181227 - Flags: review?(dbaron)
(Reporter)

Updated

12 years ago
Attachment #181227 - Flags: superreview?(dbaron)
Attachment #181227 - Flags: superreview+
Attachment #181227 - Flags: review?(dbaron)
Attachment #181227 - Flags: review+
Comment on attachment 181227 [details] [diff] [review]
fix

This fixed an incremental reflow bug which happens to manifest on the Acid2
test.
Attachment #181227 - Flags: approval1.8b2?

Comment 9

12 years ago
Comment on attachment 181227 [details] [diff] [review]
fix

a=asa
Attachment #181227 - Flags: approval1.8b2? → approval1.8b2+
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.