Closed Bug 1070952 Opened 5 years ago Closed 2 years ago

Context-menu 2.0

Categories

(Add-on SDK Graveyard :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: irakli, Assigned: irakli)

References

Details

Attachments

(6 files, 3 obsolete files)

46 bytes, text/x-github-pull-request
evold
: review+
Details | Review
46 bytes, text/x-github-pull-request
evold
: review+
Details | Review
46 bytes, text/x-github-pull-request
evold
: review+
Details | Review
46 bytes, text/x-github-pull-request
evold
: review+
Details | Review
52 bytes, text/plain
evold
: review+
Details
46 bytes, text/x-github-pull-request
evold
: review+
Details | Review
Current context menu API is not optimized for simple us case, it always requires use of content scripts that is quite an overhead. In addition it's really hard to port it to E10s.

I think it's a good time to consider designing context-menu@2.0 with a simpler API that is designed and implemented with e10s in mind. Content script should not be a part of context menu API as it adds a lot of complexity and overhead for a very little benefit.
related bug 1060138, and some discussion (for why old api is bad) in bug 1058698..
OS: Mac OS X → All
Hardware: x86 → All
Priority: -- → P1
Attached file Initial review request (obsolete) —
I'm still adding more tests, but I think it's ready for initial review.
Attachment #8522488 - Flags: review?(evold)
Assignee: nobody → rFobic
Attached file Changes to toolkit/require (obsolete) —
Attachment #8524426 - Flags: review?(evold)
Comment on attachment 8524426 [details] [review]
Changes to toolkit/require

I mentioned a few changes that I'd like to see, and this also needs some tests.

Also I wonder if the modules should be unloaded somehow too, instead of just being deleted.
Attachment #8524426 - Flags: review?(evold)
Attachment #8524426 - Flags: review-
Attachment #8524426 - Flags: feedback+
Attachment #8524391 - Flags: review?(evold) → review+
Comment on attachment 8524447 [details] [review]
Utils for working with message managers

Looks good to me, but it needs tests.  Also mark the modules as unstable and add the license header please.
Attachment #8524447 - Flags: review?(evold) → feedback+
Comment on attachment 8522488 [details] [review]
Initial review request

Taking my r? off of this since we agreed to split it up in to separate pull requests.
Attachment #8522488 - Flags: review?(evold)
Comment on attachment 8524413 [details] [review]
Changes to sdk/util/sequence

lgtm!
Attachment #8524413 - Flags: review?(evold) → review+
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/62a6ad476137dc0e1acf778cce857ea80b67389d
Merge pull request #1717 from Gozala/context-menu-2-disosables@1070952

Bug 1070952 - Export `setupDisposable` and `disposeDisposable` r=@erikvold
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/29566c0e4096e5beec11d6705831b8aa1731407b
Merge pull request #1718 from Gozala/context-menu-2-sequence@1070952

Bug 1070952 - Update sequence lib to cover ES6 Symbols. r=@erikvold
Addressed review comments and have added tests.
Attachment #8525179 - Flags: review?(evold)
Attachment #8524426 - Attachment is obsolete: true
Attachment #8522488 - Attachment is obsolete: true
Added licenses and metadata + tests, re-requesting review.
Attachment #8524447 - Attachment is obsolete: true
Attachment #8525236 - Flags: review?(evold)
Comment on attachment 8525179 [details]
Changes to toolkit/require v2

I made a mentioned of edits for the comments, looks good otherwise.
Attachment #8525179 - Flags: review?(evold) → review+
Comment on attachment 8525236 [details] [review]
Utils for working with message managers V2

nice!
Attachment #8525236 - Flags: review?(evold) → review+
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/7009435ffcde9933f2d202b863a29123bcbaac45
Merge pull request #1719 from Gozala/context-menu-2-require@1070952

Bug 1070952 - Implement module reloading useful during development. r=@erikvold
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/fc3b3e0d5109b86f54fce17c7de87922aa79363e
Merge pull request #1720 from Gozala/context-menu-2-message-manager@1070952

Bug 1070952 - Utils for working with message managers. r=@erikvold
Attachment #8524456 - Flags: review?(evold) → review+
Attachment #8524463 - Flags: review?(evold) → review+
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/65cbeef3b22ad43af2108be56c873f91e339e8dd
Merge pull request #1721 from Gozala/context-menu-2-component@1070952

Bug 1070952 - Implement react style component API for UI updates. r=@erikvold
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/a5e46bb032b32264a107b2a1b4f1761c9e5a10ae
Merge pull request #1751 from Gozala/re-context-menu@2

Bug 1070952 - ContextMenu@2 r=@erikvold
Duplicate of this bug: 646357
Duplicate of this bug: 773914
Duplicate of this bug: 936189
Duplicate of this bug: 882900
Duplicate of this bug: 1014453
Duplicate of this bug: 824182
https://bugzilla.mozilla.org/show_bug.cgi?id=1399562
Status: REOPENED → RESOLVED
Closed: 5 years ago2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.