508115 - some drop down fields misaligned in options panel

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[(mostly gone) XtC4UaLL [:xtc4uall]]

in Fx options panel several drop down fields are misaligned, e.g.
- content/fonts & colors/advanced: size fields
- privacy/history/custom settings/third party cookies: keep until drop down

also see the screenshots mentioned in http://forums.mozillazine.org/viewtopic.php?p=7139735#p7139735

regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=77a8228f2951&tochange=616ba2372e52

Comment 1

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

Clearly a blocker...

Comment 2

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

This is the new code in bug 506615 that calls CreateViewForFrame for the children of deck frames. When the deck child contains a popup menu, this is causing menupopup frames, which already have views, to have those views reparented from the root view to the deck child's view. This breaks things. I'm not quite sure of the best fix. We could go back to having FrameNeedsView check for a deck parent but I didn't like polluting that code with decks.

Comment 3

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

We probably should fix things so that view reparenting doesn't mess with menupopup views in general. I bet there's a related bug where reparenting views due to reflow messes up a menupopup child.

Comment 4

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

Isn't all this temporary anyway?  (Seems easy enough to put back something like the old code?)

Comment 5

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

Attachment: testcase

This testcase asserts twice, once for the menulist-in-deck situation and once for the menulist-in-wrapping-span-with-transform situation. The popups appear in the wrong places, but I can't find a way to test the popup position; boxObject.screenX/Y lie and give us the right position.

Although this is a little obscure I think we should fix both bugs.

Comment 6

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

Attachment: fix

This stops us from reparenting a popup's view which needs to always have the root as its parent.

Comment 7

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

This is a P1 1.9.2 blocker, thus blocking Firefox 3.6a1 - any ETA on landing? (not sure this really needs to be a P1, though)

Comment 8

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

It's ready to land, I'll land it today if I can.

Comment 9

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

http://hg.mozilla.org/mozilla-central/rev/14a9da98c645

But I had to disable the test, it was failing:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1249443093.1249445036.10296.gz
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/mozilla-central-linux-unittest-everythingelse/build/reftest/tests/layout/generic/crashtests/508115-1.html | timed out waiting for onload to fire

http://hg.mozilla.org/mozilla-central/rev/a147eec34099

Comment 10

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

The popups don't show up in the reftest harness. This probably needs to be rewritten as a mochitest.

Comment 11

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

Attachment: fixed test

This fixes the test, making it a mochitest.

Comment 12

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

Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed on OS X and Windows with builds like with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a2pre) Gecko/20090903 Namoroka/3.6a2pre (.NET CLR 3.5.30729) ID:20090903051734

Roc, is the test ready to land?

Comment 14

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

Test checked in:
http://hg.mozilla.org/mozilla-central/rev/be0e542d387a