Closed
Bug 1151610
Opened 9 years ago
Closed 9 years ago
Manage the case where two extensions fight over the same command
Categories
(DevTools :: Console, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: fayolle-florent, Assigned: fayolle-florent)
References
Details
Attachments
(1 file, 1 obsolete file)
11.82 KB,
patch
|
bgrins
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 Build ID: 20150330154247 Steps to reproduce: After bug 1143497 (which introduces the WebConsoleCommands API), the conflicts between two extensions for a same command introduces inconsistent behavior. See: https://pastebin.mozilla.org/8828974 Conversation with :bgrins about this: https://pastebin.mozilla.org/8828979 Florent
Assignee | ||
Updated•9 years ago
|
Component: Untriaged → Developer Tools: Console
OS: Linux → All
Hardware: x86_64 → All
Summary: Manage the case where two extensions are fighting over the same command → Manage the case where two extensions fight over the same command
Comment 1•9 years ago
|
||
My thinking right now is that any call to unregister() should restore the default behavior, if any. So anything that "built in" (i.e. registered by the tools from utils.js) will be restored if that key is unregistered. This will both prevent the built in commands from being removed with no replacement, and also make it easy for addons to clean up after themselves (just call unregister() on any commands you've added).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 2•9 years ago
|
||
I am OK with your idea, Brian. I would also introduce a new API to easily and properly override a command. See https://pastebin.mozilla.org/8829109 Florent
Assignee | ||
Comment 3•9 years ago
|
||
Here is the current status of my work. Florent
Comment 5•9 years ago
|
||
(In reply to fayolle-florent from comment #4) > Honza, what do you think? > > Florent I also vote for restoring the default (command) behavior if an extension calls unregister (even if there is yet another extension overriding the same command). Honza
Flags: needinfo?(odvarko)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8589848 [details] [diff] [review] 1151610.patch Alright. So :brian, could you review the patch (which only implements your idea). Florent
Attachment #8589848 -
Flags: review?(bgrinstead)
Comment 7•9 years ago
|
||
Comment on attachment 8589848 [details] [diff] [review] 1151610.patch Review of attachment 8589848 [details] [diff] [review]: ----------------------------------------------------------------- LGTM ::: toolkit/devtools/webconsole/utils.js @@ +1570,5 @@ > * @param {string} name The name of the command > */ > unregister: function(name) { > this._registeredCommands.delete(name); > + if (this._originalCommands.has(name)) Nit: brackets around the if block
Attachment #8589848 -
Flags: review?(bgrinstead) → review+
Assignee | ||
Comment 8•9 years ago
|
||
Done. Florent
Attachment #8589848 -
Attachment is obsolete: true
Attachment #8598188 -
Flags: review?(bgrinstead)
Updated•9 years ago
|
Attachment #8598188 -
Flags: review?(bgrinstead) → review+
Updated•9 years ago
|
Assignee: nobody → fayolle-florent
Status: NEW → ASSIGNED
Comment 9•9 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec39444222e7
Assignee | ||
Comment 10•9 years ago
|
||
Everything looks good after the try push. Florent
Keywords: checkin-needed
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/21b92a74a01f
Keywords: checkin-needed
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/21b92a74a01f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•