Closed Bug 1393497 Opened 7 years ago Closed 7 years ago

[nosdk] Valence addon still using sdk dependencies

Categories

(DevTools :: General, enhancement, P1)

enhancement

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: jdescottes, Assigned: zer0)

References

Details

(Whiteboard: [nosdk])

Attachments

(1 file)

See https://github.com/mozilla/valence/search?utf8=%E2%9C%93&q=sdk&type= 

We need to update the Valence addon to no longer rely on the SDK classes.
Whiteboard: [nosdk]
Flags: qe-verify-
Whiteboard: [nosdk] → [reserve-nosdk]
Assignee: nobody → zer0
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [reserve-nosdk] → [nosdk]
Target Milestone: --- → Firefox 57
I double checked that Valence is not shipped with DevEdition; but it's just installed the first time WebIDE is opened in any Firefox versions, despite what the Valence's repo said.

It seems to me, that the quickest way to avoid that, it's just set this pref to `false`:

http://searchfox.org/mozilla-central/source/devtools/client/webide/webide-prefs.js#7

Or remove the pref, and hard code this to `false`:

http://searchfox.org/mozilla-central/source/devtools/client/webide/content/webide.js#93

Since AFAIK we want to remove WebIDE code anyway, it doesn't seems to me valuable refactoring WebIDE code to remove any Valence / Adapters reference.

Alex, since you've more insight about adb-helper / valence than me, what do you think? Seems reasonable proceed in this way?
Flags: needinfo?(poirot.alex)
Removing the pref and all code that use it sounds like a good approach.

But that's not all the entry points for Valence.
There is also an entry for it in "extra components" panel,
in WebIDE select "Project > Manage extra components" menu entry,
you will see "Tools adapters Add-on" entry.
This should be implemented here:
  http://searchfox.org/mozilla-central/source/devtools/client/webide/content/addons.js#85-94

Otherwise a code search on "adapters" should help identifying all things related to valence:
  http://searchfox.org/mozilla-central/search?q=adapters&case=false&regexp=false&path=devtools%2Fclient%2Fwebide

Also we may put some messages on MDN to say it is no longer maintained:
https://developer.mozilla.org/en-US/docs/Tools/WebIDE/Troubleshooting#Connecting_to_other_browsers_(Chrome_Safari_iOS)
https://developer.mozilla.org/en-US/docs/Tools/Valence
Flags: needinfo?(poirot.alex)
See Also: → 1397274
Alex, I filed bug 1397274 for the docs, thanks!

About the patch, I run the webIDE tests (devtools/client/webide/test/) and I got a couple of errors, especially one modal saying: "Can’t fetch the add-on list: JSON not available, CDN error: [object ProgressEvent], storage error: Empty cache for devtools.webide.addonsURL" but since I got the same results in mozilla central without the patch updated, I believe is not related to this changes.

Build and run WebIDE seems fine, but I don't know the codebase, so let's double check to see if I missed anything else.
Comment on attachment 8905303 [details]
Bug 1393497 - remove Valence from WebIDE;

https://reviewboard.mozilla.org/r/177100/#review182712

Thanks Matteo for this cleanup, it looks good.

r+ assuming you get a green try.
Attachment #8905303 - Flags: review?(poirot.alex) → review+
Triggered Autoland from Review Board.
Pushed by mferretti@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9815926c3bc1
remove Valence from WebIDE; r=ochameau
https://hg.mozilla.org/mozilla-central/rev/9815926c3bc1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.