Not doing so leaves us vulnerable to bugs like bug 538062. But fixing it breaks accessibility tests. See for example http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1273714154.1273715739.12196.gz whch you get by uncommenting the line here if (textStyleContext) { newFrame = NS_NewTextFrame(presContext->PresShell(), textStyleContext); if (newFrame) { // initialize the text frame newFrame->Init(mTextContent, parentFrame, nsnull); // mTextContent->SetPrimaryFrame(newFrame); } } } in nsGfxButtonControlFrame.cpp

Hmm. The primary frame set should probably be in nsCSSFrameConstructor::CreateAnonymousFrames where we call CreateFrameFor (that would catch the combobox case too, right?). Sorry I missed that codepath.... :(

Though I guess for the combobox case the returned frame doesn't have the given content as its content... so nevermind. :(

Mats caught it. The only problem is that fixing it breaks accessibility tests because the accessibility code doesn't expect to find a text accessible child of buttons.

David Bolter, do you mind working on this? All you need to do is uncomment the line in comment #0 and then fix the accessibility tests.

Yep, I'll take a look real soon.

I want to understand this change better, can I have access to bug 538062?

Done.

Blocking 1.9.3 final+.

OK please go ahead and reverse a908be341373 and take this patch. We uncovered some questions with our accessibility name computation that we'll follow up with in 567203. Thanks for checking in with me.

Thanks!

Checked in: http://hg.mozilla.org/mozilla-central/rev/c6816e9bb38f http://hg.mozilla.org/mozilla-central/rev/d40ae9315e5d