Closed
Bug 1359085
Opened 8 years ago
Closed 7 years ago
Extract devtools/server/content-globals from devtools
Categories
(DevTools :: General, enhancement, P3)
DevTools
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
Attachments
(1 file)
addon-sdk/source/lib/sdk/loader/sandbox.js relies on devtools/server/content-globals . It is actually the only consumer for this class in mozilla-central today (except for tests, including one in devtools: devtools/shared/tests/mochitest/test_devtools_extensions.html) Since DevTools are being removed from mozilla central, content-globals.js should be moved outside of devtool to addon-sdk.
Assignee | ||
Comment 1•8 years ago
|
||
Simply moving the module to addon-sdk is not enough. content-globals.js relies on being loaded by the devtools loader in order to : - be able to inject globals - check if we are loaded in a worker (relying on the isWorker global)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•8 years ago
|
||
I am actually not sure if the globals set using content-globals are ever used. The only call site for getContentGlobals used to be in devtools/server/actors/script.js but was removed in https://bugzilla.mozilla.org/show_bug.cgi?id=1039952 (I searched both in mc and addons code) I will do a try run removing this code and see what happens.
Assignee | ||
Comment 3•8 years ago
|
||
Try seems green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3864dc81e1063fc35cf26db29cf9f9e2af3d8340 Alex, do you have any insights about content-globals? As I said getContentGlobals is never used, so I think we should consider removing this completely.
Flags: needinfo?(poirot.alex)
Comment 4•8 years ago
|
||
This is related to Addon-SDK Content scripts. It was to allow debugging them. Seing them in the debugger. I'm not sure this feature still work. May be the platform code involved in bug 1039952 ensure they are still covered. It is possible this code replaces content-globals: http://searchfox.org/mozilla-central/source/devtools/server/actors/tab.js#533-546 You can see this pattern appear in SDK test: http://searchfox.org/mozilla-central/source/addon-sdk/source/test/test-page-mod-debug.js#35-37 In any case, I confirm, this code looks useless. test-page-mode-debug.js be kept, but you can remove the logic around content-globals, test_devtools_extensions.html could be completely removed with content-globals.js, sandbox.js should be pruned from content-globals logic
Flags: needinfo?(poirot.alex)
Comment 5•8 years ago
|
||
Triaging dt-addon blockers as P3/enhancement since we're not using priorities for this project (filter on CLIMBING SHOES).
Severity: normal → enhancement
Priority: -- → P3
Comment hidden (mozreview-request) |
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8863362 [details] Bug 1359085 - remove unused devtools util content-globals.js; https://reviewboard.mozilla.org/r/135126/#review138932 Thanks for this patch!
Attachment #8863362 -
Flags: review?(poirot.alex) → review+
Comment 8•8 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 04ac52fd322c -d a86584f1364d: rebasing 393597:04ac52fd322c "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip) merging devtools/shared/tests/mochitest/chrome.ini warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Assignee | ||
Comment 9•7 years ago
|
||
rebased try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=984ad385480d7a9f013b08881438e8445f936d9a
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 1d2200ea18a8 -d c8e67ddc6f34: rebasing 393895:1d2200ea18a8 "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip) merging devtools/shared/tests/mochitest/chrome.ini warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment 13•7 years ago
|
||
Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d69be43cf123 remove unused devtools util content-globals.js;r=ochameau
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d69be43cf123
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•