View | Show/Hide | Site Navigation Bar and associated sub-menu items don't have access keys defined. 2002-03-11-04 Win98 (0.9.9)
Created attachment 75872 [details] [diff] [review] Patch Defines access keys as follows: View | Show/Hide | S_i_te Navigation Bar (S is used by Status Bar, and N by Navigation Toolbar) | _S_how Always | Show _O_nly as Needed | _H_ide Always
Also consider Show Only as _N_eeded. I can make another patch if that's determined to be better.
i think i'd prefer site na&vigation bar
Why exactly? I would think that because "site" is the distinguishing word, it would get the emphasis. Granted, an underlined "i" doesn't stand out very well though.
Gerv, shame on you for not putting the accesskeys in in the first place - very naughty, please don't do it again or you'll have the wrath of the accessibility world upon you. It's your bug. Maybe Greg is still around to do it for you.
Comment on attachment 75872 [details] [diff] [review] Patch Greg, if you're still around, we avoid putting accesskey on lower case i or l, because they are thin letters so the underline is hard to see. The full xul accesskey faq is at http://www.mozilla.org/projects/ui/accessibility/accesskey.html
Created attachment 95512 [details] [diff] [review] Patch 2 Wow, I forgot about this bug altogether. This patch sets the accesskeys to: Site N_a_vigation Bar > _S_how Always Show _O_nly as Needed _H_ide Always
Huh? I didn't implement this menu item. sballard did. I just checked it in for him. (Yes, perhaps I should have spotted this in review.) Gerv
Comment on attachment 95512 [details] [diff] [review] Patch 2 r=aaronl Thanks Greg - are you interested in helping out with any of the other bugs under meta bug 129179? BTW, I'm currently getting bug 68841 reviewed, which will help a lot, because XUL radios and checkboxes don't currently get their accesskeys underlined.
Comment on attachment 95512 [details] [diff] [review] Patch 2 sr=jag
Created attachment 96265 [details] [diff] [review] Patch 2.1 Ack. PatchMaker didn't do a very good filename translation on that one. This one should work.
Comment on attachment 96265 [details] [diff] [review] Patch 2.1 Carrying over reviews.
Hmm, why still no lines in the patch like: Index: blah ============== Maybe there's some way to apply this diff that I don't know? patch -p0 <patchname won't work
Hmm... I'll try to look into this. There weren't any changes to the underlying files since the patch was made, and the patch to bug 139624 checked in okay, which was also made in PatchMaker and has the same style.
Created attachment 96272 [details] [diff] [review] Patch 2.2 Okay, this one was pulled from CVS and done without PatchMaker.
Comment on attachment 96272 [details] [diff] [review] Patch 2.2 Maybe it was the date format. The PatchMaker patch seems to use a different one than diff. The PatchMaker patch from the other bug doesn't use that date format either, so maybe a bug got added to the new version.
Checked in. I manually edited the lines starting "Index: " to include full paths, and then the patch applied. I think I've heard this is a problem with cvs-mirror's old CVS server software, but I could be wrong. Gerv, are patches created by patchmaker all suffering from these problems?
Aaron, no those diffs are fine. You just need to cd into the directory where the patch was created. Since there are no directories in the index lines, the patch was created in the same directory as the file. It will apply cleanly from within mozilla/xpfe/browser/resources/content/
As the man says. Patch Maker users should create all patches from the top-level Mozilla directory, i.e. using pma from that directory, and running pmd in it. Gerv
Actually, that's what I did for patch 2 and 2.1. Compare their date lines to the first patch. Do you think that would cause the problem?
mass remove verifyme requests greater than 4 months old