Bug 86847 - Web Inspector: Support hierarchical context menus
Summary: Web Inspector: Support hierarchical context menus
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on: 86625
Blocks: 86630
  Show dependency treegraph
 
Reported: 2012-05-18 05:46 PDT by Alexander Pavlov (apavlov)
Modified: 2012-05-24 07:53 PDT (History)
14 users (show)

See Also:


Attachments
Patch (14.66 KB, patch)
2012-05-22 10:10 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
Patch (21.78 KB, patch)
2012-05-23 05:09 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
Patch (36.47 KB, patch)
2012-05-24 04:56 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>