Bug 9624 – REGRESSION: After ctrl-clicking in a EMPTY input or textarea field, the contextual menu shows "Search in Google" and "Search in Spotlight" as active menu items

• Component: Forms
• Status: RESOLVED FIXED

[Chris Petersen]

This sounds related to the http://bugzilla.opendarwin.org/show_bug.cgi?id=9580. 
 Ctrl-clicking in a empty input or textarea field still shows ""Search in Google" and "Search in Spotlight" as active items in the contextual menu. These menu items are active but do nothing when selected.

 * STEPS TO REPRODUCE
1. With a TOT webkit NB, go to http://www.apple.com/macosx/feedback/
2. Ctrl-click in the empty Name field or Comments field. Notice both menu items mentioned above are active but aren't functional.

* RESULTS
Both "Search in Google" and "Search in Spotlight" should be dimmed in the contextual if I cntrl-click in a empty field.  

[Chris Petersen]

This issue is filed as <rdar://problem/4604650> in Radar.

[David Kilzer (:ddkilzer)]

With debug builds of WebKit, this causes an assertion failure.

Steps to reproduce:

1. Open Safari+WebKit with a recent ToT debug build of WebKit.
2. Load a page with a textarea or text field on it.
3. Click in the textarea or text field to give it focus.
4. Right-click (or ctrl-left-click) somewhere on the page (but NOT on a link--that's a different bug that's getting filed next).

Expected results:

Contextual menu is pulled up with nominal options.

Actual results:

Assertion failure with debug builds of WebKit.  Release builds follow the behavior in Comment #0.

Notes:

Reproduced with Safari 2.0.4 (419.3) and a locally-built WebKit r15125 (with an in-progress patch for Bug 9179) on Mac OS X 10.4.7 (8J135/PowerPC).

Assertion:

ASSERTION FAILED: [[menuItems objectAtIndex:1] isSeparatorItem] (/Users/ddkilzer/Projects/Cocoa/WebKit/WebKit/WebView/WebView.m:723 -[WebView(WebPrivate) _menuForElement:defaultItems:])

Stack trace:

Date/Time:      2006-07-01 21:10:31.228 -0500
OS Version:     10.4.7 (Build 8J135)
Report Version: 4

Command: Safari
Path:    /Applications/Safari.app/Contents/MacOS/Safari
Parent:  bash [399]

Version:        2.0.4 (419.3)
Build Version:  1
Project Name:   WebBrowser
Source Version: 4190300

PID:    2914
Thread: 0

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xbbadbeef

Thread 0 Crashed:
0   com.apple.WebKit         	0x0039aa68 -[WebView(WebPrivate) _menuForElement:defaultItems:] + 888 (WebView.m:723)
1   com.apple.WebKit         	0x0036eee8 -[WebHTMLView menuForEvent:] + 404 (WebHTMLView.m:2533)
2   com.apple.AppKit         	0x93b34c5c -[NSView rightMouseDown:] + 68
3   com.apple.AppKit         	0x93a06404 -[NSControl _rightMouseUpOrDown:] + 440
4   com.apple.AppKit         	0x9374afa0 -[NSWindow sendEvent:] + 6424
5   com.apple.Safari         	0x00021734 0x1000 + 132916
6   com.apple.AppKit         	0x936f38d4 -[NSApplication sendEvent:] + 4172
7   com.apple.Safari         	0x00021238 0x1000 + 131640
8   com.apple.AppKit         	0x936ead10 -[NSApplication run] + 508
9   com.apple.AppKit         	0x937db87c NSApplicationMain + 452
10  com.apple.Safari         	0x0005c77c 0x1000 + 374652
11  com.apple.Safari         	0x0005c624 0x1000 + 374308

[David Kilzer (:ddkilzer)]

Bug 9680 may be closely related (or a duplicate) of this bug, but the behavior is slightly different.

[Darin Adler]

(In reply to comment #0)
> Both "Search in Google" and "Search in Spotlight" should be dimmed in the
> contextual if I cntrl-click in a empty field.  

I'm don't think that's correct. Many items that dim on normal menus instead are omitted completely from contextual menus when they don't apply. For example, both of these items are omitted entirely with the older versions of WebKit in this case, and I think that's the correct behavior.

So we want to omit these items rather than dimming them.

[Darin Adler]

The assertion failure occurs only with Tiger Safari, not TOT Safari, so people on the Safari team investigating this bug will see different results.

[Darin Adler]

The code that controls this is in WebDefaulContextMenuDelegate -- the comments there scared me away from making a change to remove the items that don't apply. Disabling the commands would be done in -[WebView validateUserInterfaceItem:]. That would be relatively easy.

[Darin Adler]

Although this is a regression, I'm not sure it's serious enough to qualify as a P1.

[Joost de Valk (AlthA)]

Adding GoogleBug keyword in one big change.

[Anders Carlsson]

Created attachment 9415 [details]
Patch

Here's a patch. Darin, is there something wrong with doing it this way?

[Darin Adler]

Comment on attachment 9415 [details]
Patch

Seems to me that the thing we want to check here is not "is the item selected" but rather "is there any text selected in this item", but I think we should land this and refine later after more testing.

r=me

[Anders Carlsson]

Landed in r15396