371620 - Some selects have stopped working

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Eli Friedman]

From comments in bug 370379:

"some pull-down menu, <select><option>, does not work after this checkin."
"I cannot specify it, but one is in edit section of movable type (free Blog
tool)."
"one more sample.

http://roter-baum.de/spenden"

Patch: add
  if (aFrame->GetStyleContext()->GetPseudoType() ==
      nsCSSAnonBoxes::scrolledContent)
    return PR_TRUE;

to nsContainerFrame::FrameNeedsView.  (Not entirely sure why this works, or why it's needed.)

I'll make a diff later.

Comment 1

[Jeff Walden [:Waldo]]

Can you include a test (reftest or mochitest, whichever's easier to use to reproduce the bug) for this in the diff as well?

Comment 2

[Eli Friedman]

(In reply to comment #1)
> Can you include a test (reftest or mochitest, whichever's easier to use to
> reproduce the bug) for this in the diff as well?

I'm not sure if there's any way to test this... reftests don't capture popup windows, and I don't think there's any way to test this with JS.

Comment 3

[Eli Friedman]

The bug is in nsCSSFrameConstructor::ConstructFrameByDisplayType.  Basically what's happening is that the scrolled frame isn't getting a view until after its children have been constructed.  The fix in FrameNeedsView is really a dirty hack that shouldn't be needed.

The frame constructor is a mess, though, so I'm not sure how to fix it correctly.

(I think in the case of columns, this could actually end up screwing up the view heirarchy so that am block's view is a sibling of the view of the column containing it.  Yeah, a mess.)

I'll post a diff with the hack a bit later... this regression is too serious to wait for a proper fix.

Comment 4

[Eli Friedman]

Attachment: Patch

This is correct, and the view tree should be okay in all cases, I think.  The issue was that the select's view was getting the wrong parent because the parent view was created too late.  This makes scrolled frames create their view before their children.

Comment 5

[Eli Friedman]

Checked in.