Created attachment 283829 [details] Note the missing letters where the keyboard accelerators should be Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:18.104.22.168pre) Gecko/2007100503 BonEcho/22.214.171.124pre 1. New profile, start firefox 2. Tools > Options > Security > Warning Message > Settings Expected: - All tickboxes should have a keyboard accelerator, and no letters should be missing. Actual: - Only the first tickbox has a keyboard accelerator; where the others should have one there is nothing, so the sentences read silly. eg: "I ubmit information that's not encrypted" or "I eave an encrypted page for one that isn't encrypted". Works in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:126.96.36.199) Gecko/2007091417 Firefox/188.8.131.52 with a new profile. Found by Warduke at http://forums.mozillazine.org/viewtopic.php?p=3085777#3085777
works: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:184.108.40.206pre) Gecko/2007093004 BonEcho/220.127.116.11pre broken: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:18.104.22.168pre) Gecko/2007100103 BonEcho/22.214.171.124pre Checkins to module AviarySuiteBranchTinderbox on branch MOZILLA_1_8_BRANCH between 2007-09-30 03:00 and 2007-10-01 04:00 : http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-09-30+03&maxdate=2007-10-01+04&cvsroot=%2Fcvsroot So I will guess a regression of bug 267833, bug 373756, bug 394676 or bug 394014. CC'ng bz since he seemed to have a lot to do with those bugs.
Haven't been able to look into this deeply, but I don't know of anything special about that dialog off-hand. It certainly doesn't seem to differ from advanced-scripts.xul in any significant way (in fact I think I may have copied that file when I added this one).
It looks like we fail to find the primary frame for the <span> when inserting the textnode under it in <method name="wrapChar"> in text.xml. So we never construct a textframe for that one letter, and the rest follows. Looking into why we don't find the frame.
Created attachment 283948 [details] [diff] [review] Fix This bug is a result of two things. First, the patch for bug 267833 changed the ordering of constructor firing and layout under InitialReflow. As a result, without this patch on branch we do the initial layout, _then_ fire XBL constructors. In the case of this dialog, that causes the text in the labels to wrap, so that the frame tree for one of the labels looks something like this right after we've inserted the child span node: (nsAreaFrame *) 0x88a315c (nsXULElement *) 0x8b15cb0 (nsInlineFrame *) 0x88a2f8c (nsHTMLSpanElement *) 0x88a4358 (nsTextFrame *) 0x8b17b28 (nsTextNode *) 0x8b15d38 (nsInlineFrame *) 0x8b2161c (nsHTMLSpanElement *) 0x88a4358 (nsContinuingTextFrame *) 0x8b215d8 (nsTextNode *) 0x8b15d38 (nsInlineFrame *) 0x8b216c8 (nsHTMLSpanElement *) 0x88a4358 (nsContinuingTextFrame *) 0x8b21684 (nsTextNode *) 0x8b15d38 (nsInlineFrame *) 0x8b21774 (nsHTMLSpanElement *) 0x88a4358 (nsContinuingTextFrame *) 0x8b21730 (nsTextNode *) 0x8b15d38 (nsInlineFrame *) 0x8b21820 (nsHTMLSpanElement *) 0x88a4358 (nsContinuingTextFrame *) 0x8b217dc (nsTextNode *) 0x8b15d38 (nsInlineFrame *) 0x8b2195c (nsHTMLSpanElement *) 0x8b26f98 (nsTextFrame *) 0x8b21994 (nsTextNode *) 0x8b29e08 Here the nsAreaFrame is the frame for the XUL element, all the nsInlineFrames that are kids of it are frames for the anonymous span the XBL binding inserts, the text frames are for the text, and the child nsInlineFrame near the end there is the span our constructor just inserted (the one we're using to wrap the accesskey). Now we append a textnode to this inner span. At that point, we need the primary frame for that <span>, we go to look for it, end up with the first textframe there as our primary frame hint (which is correct), and get bitten by bug 334829, because of the non-isomorphism of the frame and content trees due to XBL in this case. So we don't find the frame for the <span>, and never create a frame for the text, with causes this bug. The patch is twofold. The frame constructor changes are just the patch for bug 334829, which I think we should just fix on branch. It's been baking on trunk for a long time, is conceptually the right thing to do, and is very safe, in my opinion. The presshell changes are a backport of part of the fix for bug 377119 and preserve the "constructors fire before reflow" ordering we used to have in InitialLayout. The presshell part of the patch is a little riskier (though again I think it's very much sound), but I think we need it to prevent other things that might depend on the ordering there from biting us. Either change fixes this particular bug, but I do think we should take both on branch. If we want to be a tiny bit safer, we could drop the restyle processing from the presshell diff; roc, what do you think?
Both those bugs have tests in the tree, so no need for separate test here, I think.
Comment on attachment 283948 [details] [diff] [review] Fix let's do this
Comment on attachment 283948 [details] [diff] [review] Fix approved for 126.96.36.199, a=dveditz for release-drivers
Comment on attachment 283948 [details] [diff] [review] Fix This needs to land on MOZILLA_1_8_BRANCH for future releases AND the GECKO181_20071004_RELBRANCH for the 188.8.131.52 release. If it helps I can do the merge over to the relbranch for you. Please ping me if you'd like me to.
Checked in on both those branches.
verified fixed 184.108.40.206 using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11) Gecko/2007100816 Firefox/18.104.22.168 and Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:22.214.171.124) Gecko/2007100816 Firefox/126.96.36.199 (both are the Firefox 188.8.131.52 RC 2 Builds). The Keyboard Accelerators are now back on 1.8. Adding verified keyword
verified fixed 184.108.40.206 using the steps to reproduce from steve and using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:220.127.116.11pre) Gecko/2007111203 BonEcho/18.104.22.168pre The Keyboard Accelerators are okay, adding verified keyword
This is fixed, since it's a branch-only bug and landed on branches.