871338 - Layout problem on certain site since landing of Bug 828312

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Alice0775 White]

Attachment: archive zip (Open index.html )

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/d68224f5325b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130512 Firefox/23.0 ID:20130512030908

Zlip792 reported the problem (See http://forums.mozillazine.org/viewtopic.php?p=12855107#p12855107 )

Steps To Reproduce:
1. Open index.html (Alternative, open URL and logged in)
2. Wait completion of loading page
3. Mouse move over around comments

Actual Results:
"Reply" button is shifted to next line

Expected Results:
"Reply" button should not move.

Regression Window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/4ff8aa4a7295
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130511 Firefox/23.0 ID:20130511071545
Bad:
http://hg.mozilla.org/mozilla-central/rev/d68224f5325b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130511 Firefox/23.0 ID:20130511223226
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4ff8aa4a7295&tochange=d68224f5325b

Regression Window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/244595f01a56
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130510 Firefox/23.0 ID:20130510172744
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b7330cb50416
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130510 Firefox/23.0 ID:20130510173544
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=244595f01a56&tochange=b7330cb50416

Regressed by: Bug 828312

Comment 1

[Alice0775 White]

In local buidl,
Last Good: bf5fd9db9078
First Bad: 09552b219c3a

Comment 2

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

Attachment: simplified testcase

Comment 3

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

Attachment: simplified testcase with minimal whitespace

some of the spaces are necessary

Comment 4

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

From comment 1, regressing changeset is http://hg.mozilla.org/mozilla-central/rev/09552b219c3a .

Calling MarkLineDirty on all lines when NS_BLOCK_LOOK_FOR_DIRTY_FRAMES is set does *not* make the bug go away.

But adding NS_FRAME_IS_DIRTY to the block when NS_BLOCK_LOOK_FOR_DIRTY_FRAMES is set *does* make the bug go away.

Setting NS_BLOCK_LOOK_FOR_DIRTY_FRAMES on this in nsBlockFrame::ChildIsDirty (in addition to parent) does *not* make the bug go away.

The problem is that the MarkLineDirty needs to happen before GetPrefWidth/GetMinWidth do their work.

Comment 5

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

Attachment: Handle NS_BLOCK_LOOK_FOR_DIRTY_FRAMES lazy marking in GetMinWidth and GetPrefWidth in addition to Reflow.

Comment 6

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

And I checked that the reftest fails without the patch and passes with it.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 748653 [details] [diff] [review]
Handle NS_BLOCK_LOOK_FOR_DIRTY_FRAMES lazy marking in GetMinWidth and GetPrefWidth in addition to Reflow.

r=me

Comment 8

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

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

Comment 9

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

Comment on attachment 748653 [details] [diff] [review]
Handle NS_BLOCK_LOOK_FOR_DIRTY_FRAMES lazy marking in GetMinWidth and GetPrefWidth in addition to Reflow.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 828312
User impact if declined: incorrect page layout
Testing completed (on m-c, etc.): will be on mozilla-central soon; tested that it fixes the page in question
Risk to taking this patch (and alternatives if risky): reasonably low (I think it's fine for the first week of aurora); alternative is to back out patch 3 in bug 828312
String or IDL/UUID changes made by this patch:none

Comment 10

[Ed Morley [:emorley]]

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

Comment 11

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/09e58f631159

Comment 12

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 23 beta 7:

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0
Build ID: 20130718163513

Comment 13

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 24 beta 2:

Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Build ID: 20130812173056