388361 - dropdowns in font preferences broken after switching language groups

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

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

After switching language groups in the font preferences in Firefox, the dropdowns for choosing fonts are empty.  This is a regression from the XUL popup rewrite (bug 279703), but it wouldn't surprise me if what's needed is a JS fix in fontbuilder.js.

Steps to reproduce:
 1. start Firefox on a clean profile
 2. Edit -> Preferences
 3. choose the "Content" tab
 4. click the "Advanced" button in the "Fonts & Colors" section
 5. drop down the menu for "Sans-serif:"
 6. change the "Fonts for" dropdown from "Western" to "Simplified Chinese" and then back to "Western" (one change is enough, but this gets back to a state where you know what the good state is)
 7. drop down the menu for "Sans-serif:" again

Expected results:
 5. menu populated with list of fonts
 7. likewise

Actual results:
 5. as expected
 7. menu doesn't drop down

Bug present in:
 * Linux nightly 2007-07-05-04-trunk
 * Linux debug build, evening of 2007-07-15

Works correctly in:
 * Linux nightly 2007-07-04-04-trunk

Comment 1

[Neil Deakin]

I also get this assertion:

###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file /builds/trunk-working/mozilla/layout/generic/nsFrame.cpp, line 708

Comment 2

[Neil Deakin]

Attachment: testcase

This is a minimal testcase showing the bug. As I suspected, removing the popup and readding it causes the problem. One solution for this particular bug is to change fontbuilder.js to not do that and just remove the items instead. I'll investigate the real issue, but the fontbuilder.js change would be good optimization also.

The assertion seems to be a different bug, caused by having the menulist inside a groupbox in some certain way.

Comment 3

[Neil Deakin]

Attachment: when a frame is added or removed, reset the cached last prefered size

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272651 [details] [diff] [review]
when a frame is added or removed, reset the cached last prefered size

Looks reasonable.  I assume that this compiles to the same thing as using SizeTo()?

Please check in the test as well.

Comment 5

[Neil Deakin]

Attachment: like so?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 272777 [details] [diff] [review]
like so?

Looks great.

Comment 7

[[e-mail]]

(In reply to comment #2)
>This is a minimal testcase showing the bug. As I suspected, removing the popup
>and readding it causes the problem. One solution for this particular bug is to
>change fontbuilder.js to not do that and just remove the items instead. I'll
>investigate the real issue, but the fontbuilder.js change would be good
>optimization also.
I might not be looking at the same version of fontbuilder.js but it's conceptually simpler to remove the whole popup and create a new popup and add the new items to that than to remove all the old items.