Closed Bug 1271399 Opened 8 years ago Closed 8 years ago

Need to shim protocol.js move for add-ons


(DevTools :: General, defect, P1)



(firefox48 unaffected, firefox49+ fixed)

Firefox 49
49.2 - May 23
Tracking Status
firefox48 --- unaffected
firefox49 + fixed


(Reporter: jryans, Assigned: jfong)



(Keywords: addon-compat, regression)


(2 files, 1 obsolete file)

In bug 1270173, protocol.js was moved to shared.

However, there are also add-ons that load this module to define their own actors, including the WebSocket Monitor[1] that we suggest people use on hacks.m.o.

We should create a shim module so that is can still be loaded with the old module ID as well as the new one.

Whiteboard: [devtools-html] [triage]
:ednapiranha, can you take a look at this?  Creating a small shim that lives in /devtools/server/shims and loads the module for the new ID should do it.
Flags: needinfo?(jfong)
[Tracking Requested - why for this release]: Let's make sure we land something here in 49 to maintain add-on compat.
Keywords: addon-compat
Tracking for 49 as add on compat is important.
Assignee: nobody → jfong
Flags: needinfo?(jfong)
Blocks: 1263289
Priority: P1 → P2
Attached patch Bug1271399.patch (obsolete) — Splinter Review
So I think I have most of the setup, but existing add-ons still aren't able to detect the path. I did notice that this path (EXTRA_JS_MODULES.devtools.server) points to resource://gre/modules/devtools/server/protocol.js rather than resource://devtools/server/protocol.js. 

Any idea what I might be missing here to make it point to the correct location?
Attachment #8750879 - Flags: feedback?(jryans)
Comment on attachment 8750879 [details] [diff] [review]

Review of attachment 8750879 [details] [diff] [review]:

::: devtools/server/shims/
@@ +15,5 @@
>  EXTRA_JS_MODULES.devtools += [
>      'dbg-server.jsm',
>  ]
> +
> +# Extra compatibility layer for transitional URLs used for part of 44 cycle

Update this comment to note that protocol.js was moved in 49

@@ +16,5 @@
>      'dbg-server.jsm',
>  ]
> +
> +# Extra compatibility layer for transitional URLs used for part of 44 cycle
> +EXTRA_JS_MODULES.devtools.server += [

Sorry, I gave you some bad info here! :(

Here's what should do the trick: += [ ...

Now with more dots!

::: devtools/server/shims/protocol.js
@@ +33,5 @@
> +];
> +
> +const { require } =
> +  Cu.import("resource://devtools/shared/Loader.jsm", {});
> +const module = require("devtools/shared/protocol");

Since it's a CommonJS module, I think you can do:

module.exports = require("devtools/shared/protocol");

to export everything, and then you shouldn't need `EXPORTED_SYMBOLS` for this case.
Attachment #8750879 - Flags: feedback?(jryans)
Attached patch Bug1271399.patchSplinter Review
Added your changes, but still having path issues.
Attachment #8750879 - Attachment is obsolete: true
Attachment #8750914 - Flags: feedback?(jryans)
Comment on attachment 8750914 [details] [diff] [review]

Review of attachment 8750914 [details] [diff] [review]:

Looks like the DIST_SUBDIR in the is messing this up.  I'll attach my own attempt for you to check over.
Attachment #8750914 - Flags: feedback?(jryans)
The existing files are moved into a new directory (the diff view is bad at showing this, can check the patch file directly at the attachment link).
Comment on attachment 8750927 [details] [diff] [review]
Shim for protocol.js

Yep this works for me!
Attachment #8750927 - Flags: review?(jfong) → review+
Flags: qe-verify-
Whiteboard: [devtools-html] [triage] → [devtools-html]
Keywords: checkin-needed
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Iteration: --- → 49.2 - May 23
Priority: P2 → P1
Blocks: 1277706
No longer blocks: 1263289
Blocks: 1263289
Version: unspecified → Trunk
Whiteboard: [devtools-html]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.