860370 - Layout problem of NewTab Toggle Button in about:newtab

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Alice0775 White]

Attachment: screenshot

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/9db46ddfb517
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130410 Firefox/23.0 ID:20130410031044

Steps to Reproduce:
1. Open about:newtab

Regression window:
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/09ff3bd0beb8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130409 Firefox/23.0 ID:20130409015034
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/9cb8ac3f27a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130409 Firefox/23.0 ID:20130409040635
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=09ff3bd0beb8&tochange=9cb8ac3f27a2

Suspected:
9cb8ac3f27a2	Cameron McCormack — Bug 848973 - Make nsStyleDisplay::IsBlockInside() and friends work only on style structs for the passed in frame and document them. r=roc

Comment 2

[Daniel Holbert [:dholbert]]

I'm seeing this on Linux as well.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Interestingly, the button goes back to normal when you click it, i.e. when the newtab page is disabled. All that changes is the background-position.

Comment 6

[Tobias (:Tobbi) Markus]

I suspect http://hg.mozilla.org/integration/mozilla-inbound/rev/56ceb60c5d0b to be the flawed changeset since that touches TopSites.js.

Comment 7

[Cameron McCormack (:heycam)]

I'll look into whether the bug 848973 patch caused this.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Tobias Markus (:Tobbi) [hiatus beginning January 2013] from comment #6)
> I suspect http://hg.mozilla.org/integration/mozilla-inbound/rev/56ceb60c5d0b
> to be the flawed changeset since that touches TopSites.js.

It touched TopSites.js in browser/metro/, which I believe is Win8-specific - so it wouldn't explain this manifesting on other platforms.

Anyway -- I confirmed with local backout that bug 848973's patch caused this.  (If I back out 9cb8ac3f27a2, the bug goes away.)

Comment 9

[Cameron McCormack (:heycam)]

Attachment: patch

Undo the functionality change from bug 848973 where we assumed that the styles on the scroll frame matched those from the scolled frame closely enough.  I've expanded out the IsPositioned() call like another case, to avoid hitting the newly added assertion about expected frame.

Comment 10

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

Comment on attachment 736015 [details] [diff] [review]
patch

Review of attachment 736015 [details] [diff] [review]:
-----------------------------------------------------------------

add a test

Comment 12

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9358e56ef5dd

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9358e56ef5dd

Comment 16

[g.g.michael]

I actually have no idea how to apply the patch, I just use FF Nightly because I want to look like I know what I'm doing. But boy, that was a fast fix, thanks.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

It'll be in tomorrow's nightly.