Bug 86847

Summary: Web Inspector: Support hierarchical context menus
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, apavlov, bweinstein, haraken, japhet, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 86625    
Bug Blocks: 86630    
Attachments:
Description Flags
Patch
none
Patch
none
Patch pfeldman: review+

Description Alexander Pavlov (apavlov) 2012-05-18 05:46:48 PDT
This is required to implement the pseudo-state emulation on non-selected elements
Comment 1 Alexander Pavlov (apavlov) 2012-05-22 10:10:23 PDT
Created attachment 143321 [details]
Patch
Comment 2 Pavel Feldman 2012-05-23 02:03:28 PDT
Comment on attachment 143321 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        in the Web Inspector's context menu. ContextMenuItems are also passed/stored by reference/value rather than pointer

I vaguely remember that when it was initially implemented, not all the platforms allowed treating ContextMenuItem as value type. Did this change?
Comment 3 Alexander Pavlov (apavlov) 2012-05-23 02:12:39 PDT
(In reply to comment #2)
> (From update of attachment 143321 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143321&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        in the Web Inspector's context menu. ContextMenuItems are also passed/stored by reference/value rather than pointer
> 
> I vaguely remember that when it was initially implemented, not all the platforms allowed treating ContextMenuItem as value type. Did this change?

Offline, we discovered that the platform objects represented by description are managed by the platform-specific code through the releasePlatformDescription() method.
Comment 4 Alexander Pavlov (apavlov) 2012-05-23 05:09:11 PDT
Created attachment 143538 [details]
Patch
Comment 5 Pavel Feldman 2012-05-23 09:32:36 PDT
Comment on attachment 143538 [details]
Patch

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

Make sure you implement SoftContextMenu based on the new descriptor.

> Source/WebCore/inspector/front-end/ContextMenu.js:45
> +    this._topLevelMenu = topLevelMenu;

this._contextMenu

> Source/WebCore/inspector/front-end/ContextMenu.js:51
> +    get id()

id: function()

> Source/WebCore/inspector/front-end/ContextMenu.js:56
> +    get type()

type: function()

> Source/WebCore/inspector/front-end/ContextMenu.js:95
> +        this._topLevelMenu.setHandler(item.id, handler);

Make it all private.

> Source/WebCore/inspector/front-end/ContextMenu.js:143
> +    toProtocol: function()

_buildDescriptor:
Comment 6 Alexander Pavlov (apavlov) 2012-05-24 04:56:31 PDT
Created attachment 143788 [details]
Patch
Comment 7 Pavel Feldman 2012-05-24 07:13:10 PDT
Comment on attachment 143788 [details]
Patch

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

> Source/WebCore/bindings/v8/custom/V8InspectorFrontendHostCustom.cpp:80
> +        v8::Local<v8::Value> subItems = item->Get(v8::String::New("subItems"));

subItems -> items ?
Comment 8 Alexander Pavlov (apavlov) 2012-05-24 07:53:30 PDT
Committed r118374: <http://trac.webkit.org/changeset/118374>