Bug 12134 – REGRESSION: Assertion failure and crash when right clicking selection in forms

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Matt Lilek]

In this bug report, select something in the Summary or Keywords fields and right click, you'll get the following assertion failure and crash:

ASSERTION FAILED: [[newMenuItems objectAtIndex:1] isSeparatorItem]
(/Users/matt/Code/WebKit/WebKit/WebCoreSupport/WebContextMenuClient.mm:76 getCustomMenuFromDefaultItems)

[Matt Lilek]

Created attachment 12254 [details]
Crashlog

[Darin Adler]

I tried and can't reproduce this. Maybe it's fixed already?

[Beth Dakin]

I can't reproduce this either. It's possible that it has been fixed, but I am not sure which specific check-in would have addressed this.

[Matt Lilek]

Sorry, I could have been clearer in my initial report.

I can reliably reproduce this by double clicking on "REGRESSION" in the summary field and then right clicking with r18783.  If that doesn't work, Command + A to select everything in the summary field also works - seems to be an issue with word boundaries as selecting only part of a word doesn't trigger the failure.

[Beth Dakin]

Looks like this might be fixed, then, because I still cannot reproduce with the more specific instructions (I am at r18804). Matt, could you update and try again? Then if you cannot reproduce, we can close the bug.

[Beth Dakin]

Wait never mind, I just realized that your revision number is actually very recent. I wonder why no one else can reproduce this?

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> Wait never mind, I just realized that your revision number is actually very
> recent. I wonder why no one else can reproduce this?

Assertion failures require a debug build to trip?

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> Wait never mind, I just realized that your revision number is actually very
> recent. I wonder why no one else can reproduce this?

Confirmed with locally-built DEBUG build of WebKit r18804 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037).

[Beth Dakin]

I am using a Debug build also. Dave, does your last comment mean that you *can* reproduce, or that you *cannot*?

[Alexey Proskuryakov]

I'm also getting this assertion failure (not 100% reliably).

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> I am using a Debug build also. Dave, does your last comment mean that you *can*
> reproduce, or that you *cannot*?

Sorry, I *am* able to reproduce this assertion failure.  I'm using a MBP Core 2 Duo with a Wireless Mighty Mouse (if that makes a difference).

[Alexey Proskuryakov]

(In reply to comment #11)
> I'm using a MBP Core 2 Duo with a Wireless Mighty Mouse (if that makes a difference).

A G4 here. Some debug data:

(gdb) po defaultMenuItems
<NSCFArray 0x22fe6e0>(
<MenuItem: 0x6a23b90 Search in Spotlight>,
<MenuItem: 0x6a23c00 Search in Google>,
<MenuItem: 0x6a30f10 >,
<MenuItem: 0x6a23c90 Look Up in Dictionary>,
<MenuItem: 0x6a30f80 >,
<MenuItem: 0x6a242a0 Cut>,
<MenuItem: 0x6a23d20 Copy>,
<MenuItem: 0x6a24310 Paste>,
<MenuItem: 0x6a30ff0 >,
<MenuItem: 0x6a31410 Spelling, submenu: 0x6a30380 ()>,
<MenuItem: 0x6a31870 Font, submenu: 0x6a24380 ()>,
<MenuItem: 0x6a328b0 Speech, submenu: 0x6a31f30 ()>,
<MenuItem: 0x6a330b0 Writing Direction, submenu: 0x6a32950 ()>
)
(gdb) po newMenuItems
<MenuItem: 0x22121f0 >
<MenuItem: 0x6a25490 Spelling, submenu: 0x6a1e580 ()>
<MenuItem: 0x6a1e090 >
<MenuItem: 0x6a1dcf0 Copy>

[Brady Eidson]

I found a way to reproduce this pretty reliably and helped Beth with the steps :)

[Beth Dakin]

Looks like this only happens with older version of the app. Boo.

[Adam Roben (:aroben)]

I believe the problem is that we've made new tags that have the same values as old SPI tags used to have. See the change here:
http://trac.webkit.org/projects/webkit/changeset/10008

I've got a fix on the way.

[Adam Roben (:aroben)]

Created attachment 12448 [details]
Proposed fix

[Darin Adler]

Comment on attachment 12448 [details]
Proposed fix

Looks good.

Why is there a default case in the switch statement in ContextMenu.cpp. In gcc, all the default case does is turn off the often-useful warning for enum values that have no case. If the switch value's type is not an enum, then a "default: break" case does nothing.

I think getCustomMenuFromDefaultItems would read better with an early exit rather than most of the function inside an if.

I also think it might be best to have this code only trigger on applications compiled against the old WebKit. Tim Hatcher has the details about how we can check this.

r=me

[Adam Roben (:aroben)]

Landed as r18864

[Adam Roben (:aroben)]

*** Bug 12433 has been marked as a duplicate of this bug. ***