Bug 89323 – [REGRESSION r120357] Web Inspector: no right-click context menu item 'inspect element'

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Philippe Wittenbergh]

Since r120398, I can no longer inspect an element by right-clicking on it. The context menu item is missing. Worked fine with r120350. Happens both on 10.6 and 10.7.

(I disabled all extensions just in case, but that didn't help anything)

[Alexander Pavlov (apavlov)]

I tend to close this as WONTFIX, since I cannot reproduce the issue with WebKit ToT (r120576) on Chromium. I also cannot see any change that may seem offending in the range you've provided.

Which browser/bare WebKit are you using, on what OS, and does it reproduce for you on any web page?

[Philippe Wittenbergh]

(In reply to comment #1)
> Which browser/bare WebKit are you using, on what OS, and does it reproduce for you on any web page?

Webkit nightly builds (WebKit-SVN-r12xxx.dmg, downloaded from http://nightly.webkit.org/) on OS X 10.6 and 10.7 (all up to date) as noted in my report. It happens on any and all pages.
Safari 5.1.7 works as expected.
I run my OS in French, but that shouldn't matter, I think.

[Philippe Wittenbergh]

Created attachment 148074 [details]
screenshot with WebKit nightly

Taken with WebKit-SVN-r120575.

[Philippe Wittenbergh]

Created attachment 148076 [details]
screenshot with Safari 5.1.7

(expected behaviour)

[Alexander Pavlov (apavlov)]

Timothy, can you triage/handle the issue appropriately? This seems to be broken only in the bare WebKit nightly, and I cannot see any offending change in the range reported.

[Kevin M. Dean]

Just confirming that I'm seeing the same issue with the nightlies.

[Dave]

I can confirm as I'm seeing the same issue with WebKit nightly builds.

Safari info -- Version 5.1.7 (7534.57.2, r120807)
Mac OS X -- 10.7.4

[Brad]

I've been having this same probem for the last several nightlies, and I am currently on r120914. I miss using the menu a lot. Fortunately, the magnifying glass still works. I tried turning off extensions, but it didn't help. I am also using Glims, which I have not yet tried disabling.

[Timothy Hatcher]

I'm looking into it.

[Timothy Hatcher]

<rdar://problem/11702613>

[Brad]

I just noticed in r121418 that the "Inspect Element" menu item does appear (and works) when I click on a text form field such as an input[type='text'] or a textarea. But still not on other things (such as the input[type='submit'] on this page). I don't know if this was the case or not in earlier nightlies.

(In reply to comment #11)
> I just noticed in r121418 that the "Inspect Element" menu item does appear (and works) when I click on a text form field such as an input[type='text'] or a textarea. But still not on other things (such as the input[type='submit'] on this page). I don't know if this was the case or not in earlier nightlies.

I can confirm the exact same behavior in r121382.

[Dave]

I can confirm that the issue first appeared in build r120398.  The "Inspect Element" feature was functioning correctly in build r120350.

[Timothy Hatcher]

I'm pretty sure this broke with http://trac.webkit.org/changeset/120357.

That change added a new context menu item tag, before the Inspect Element tag. This changed the enum values and thus broke any client built with an older WebKit version.

[Timothy Hatcher]

Created attachment 150337 [details]
Proposed Change

[Sam Weinig]

Comment on attachment 150337 [details]
Proposed Change

View in context: https://bugs.webkit.org/attachment.cgi?id=150337&action=review

> Source/WebCore/ChangeLog:12
> +        Fix the order of the ContextMenuAction enum to be binary compatible with
> +        older versions of WebKit.

It seems bad that we could have a binary compatibility issue like this in WebCore.  Can we change it so that the WebKit API is mapped via a switch statement to the WebCore enum so this can't happen in the future?

> Source/WebKit/mac/ChangeLog:11
> +        * WebView/WebUIDelegatePrivate.h:
> +        Add missing enums that were added in ContextMenuItem.h but left out here.

Is there anyway we can guard against this in the future?  Perhaps a switch statement without a default case somewhere?

[Timothy Hatcher]

(In reply to comment #16)
> (From update of attachment 150337 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150337&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Fix the order of the ContextMenuAction enum to be binary compatible with
> > +        older versions of WebKit.
> 
> It seems bad that we could have a binary compatibility issue like this in WebCore.  Can we change it so that the WebKit API is mapped via a switch statement to the WebCore enum so this can't happen in the future?
> 
> > Source/WebKit/mac/ChangeLog:11
> > +        * WebView/WebUIDelegatePrivate.h:
> > +        Add missing enums that were added in ContextMenuItem.h but left out here.
> 
> Is there anyway we can guard against this in the future?  Perhaps a switch statement without a default case somewhere?

That would be the best approach. But we don't really add items often at all.

[WebKit Review Bot]

Comment on attachment 150337 [details]
Proposed Change

Clearing flags on attachment: 150337

Committed r121646: <http://trac.webkit.org/changeset/121646>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kevin M. Dean]

I'm seeing no difference with r121656. Inspect element is still not appearing on anything but form fields.

[Kevin M. Dean]

Webkit doesn't seem to want to Quit in the builds where Inspect Element is missing as well. I select Quit and the window closes but the app doesn't quit and Quit in the menu grays out. Back to the last working version for me.

[Kevin M. Dean]

Seems the Quit issue is covered with Bug 90093

[Kevin M. Dean]

Tried again with r121678, and not fixed there either. Needs to be re-opened.

[Brad]

I've had the quitting problem too, but didn't think about it being related. ALSO: another place where the menu is OK, is when  there is contenteditable="true" on the item or one of its ancestors.

[Kevin M. Dean]

Per the other bug, the quitting issue has been fixed and started after this bug, so not related.

[Joseph Pecoraro]

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

[Kevin M. Dean]

(In reply to comment #26)
> *** Bug 90499 has been marked as a duplicate of this bug. ***

I noticed in the other bug you mentioned "The issue should be fixed in WebKit after r121678."

It's not fixed as I previously reported and should be re-opened unless others are seeing it as fixed.

[Joseph Pecoraro]

> I'm seeing no difference with r121656. Inspect element is still not appearing on anything but form fields.

Tim, it sounds like people are still seeing this. Any ideas?

The bug is still present in WebKit Version 5.1.7 (6534.57.2, r121952)

[Kevin M. Dean]

Someone must have taken this whole week off.

[Timothy Hatcher]

In fact, I did. I'm now back from vacation and will look into this again.

[Timothy Hatcher]

Created attachment 151502 [details]
Proposed Change for WebKit2

This needed fixed in WebKit2 also. The previous fix only fixed WebKit1 clients. I've tested this against Safari 5.1.7.

[Timothy Hatcher]

Comment on attachment 151502 [details]
Proposed Change for WebKit2

This is going to need a more complex fix. Moving the enum will fix it for Safari 5.1.7 but it will disappear for Safari 6.

[Timothy Hatcher]

Created attachment 152068 [details]
Proposed Change for WebKit2 (Round 2)

[WebKit Review Bot]

Comment on attachment 152068 [details]
Proposed Change for WebKit2 (Round 2)

Clearing flags on attachment: 152068

Committed r122520: <http://trac.webkit.org/changeset/122520>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Philippe Wittenbergh]

Thanks, the context menu works nicely now :).
@ r122535