From bug 327470: ------- Comment #2 From Chris Lawson 2006-02-18 10:12 PST [reply] ------- As with other links/searches, holding cmd while selecting this menu item should kick the search to a new tab/window, too.
Taking, patch coming.
Assignee: mikepinkerton → bugzilla
This patch has the same problem as all the other [[NSApp currentEvent] modifierFlags] calls, i.e. it won't work if you hit the modifiers after the menu is triggered. It'll be easier to fix later on if we have one thing to look for to fix, though, so I'm submitting this patch with that caveat. cl
Attachment #221997 - Flags: review?(stuart.morgan)
Comment on attachment 221997 [details] [diff] [review] uses our standard cmd/shift key toggles First, put your curly braces on a new line, makes things much easier to read. + if (modifiers & NSShiftKeyMask) + loadInBG = !loadInBG; // shift key should toggle the foreground/background pref as it does elsewhere Put this comment above the if statement. Other than those style things, looks clean to me.
Attachment #221997 - Flags: review?(stuart.morgan) → review-
As mentioned in other bugs, smfr and pink can fight out the bracing style issues in sr. Fixed the location of the comment, though. cl
Comment on attachment 222004 [details] [diff] [review] fixes comment style Again, needs a shared implementation (see bug 337958). And that will make comment 2 even easier to address later.
Attachment #222004 - Flags: review?(stuart.morgan) → review-
Fixed. Needs bug 337958 to land first to be functional. cl
Obsoleted old patch due to change in method name. This one fixes it.
Comment on attachment 242370 [details] [diff] [review] updated for new method name Looks good, except that for the search menu item, current even modifier flags won't do the trick. So you get to do my favorite thing. Use alternates! A cursory glance seems to indicate that you should just give the search context menu item kItemsNeedingOpenBehaviorAlternatesTag (in the nib). Since it already has a tag, we might have to convert the tags to be binary, and use them bitwise. Then, in |searchForSelection| if [aSender isAlternate] check [aSender keyEquivalentModifierMask], and do what you're doing now if not.
Comment on attachment 242370 [details] [diff] [review] updated for new method name Looking at this further, adding an alternate the Right Way for the context menu item will be a little bit tricky, since it requires that menu items can have two tags at once, or hacking up a solution where we have bitwise tag masks (or something). It's a big enough project for a fringe enough case that I think it should be covered in a followup bug. r=me
Comment on attachment 242370 [details] [diff] [review] updated for new method name sr=pink
Attachment #242370 - Flags: superreview?(mikepinkerton) → superreview+
Checked in on 1.8branch and trunk. The followup is bug 359694.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.