Closed Bug 1168088 Opened 10 years ago Closed 10 years ago

Remove SpiderMonkey specific JS syntax in toolkit/devtools

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: pbro, Assigned: tromey)

References

Details

Attachments

(1 file, 3 obsolete files)

There doesn't seem to be a whole lot of non-standard JS in toolkit/devtools, so it's ok to remove them all in one bug. Here's the list I found: toolkit/devtools/DevToolsUtils.js 505:18 error Unexpected token if toolkit/devtools/client/connection-manager.js 103:15 error Unexpected token for toolkit/devtools/event-emitter.js 145:68 error Unexpected identifier toolkit/devtools/gcli/commands/calllog.js 102:18 error Unexpected identifier toolkit/devtools/gcli/commands/paintflashing.js 106:24 error Unexpected identifier toolkit/devtools/gcli/commands/tools.js 24:18 error Unexpected identifier toolkit/devtools/security/auth.js 397:13 error Unexpected token for toolkit/devtools/security/tests/unit/testactors.js 49:38 error Unexpected token for toolkit/devtools/server/actors/child-process.js 41:21 error Unexpected token true toolkit/devtools/server/actors/common.js 283:9 error Unexpected identifier toolkit/devtools/server/actors/director-manager.js 486:31 error Unexpected token for toolkit/devtools/server/actors/gcli.js 263:22 error Unexpected identifier toolkit/devtools/server/actors/highlighter.js 146:14 error Unexpected token this toolkit/devtools/server/actors/inspector.js 216:14 error Unexpected token this toolkit/devtools/server/actors/root.js 263:31 error Unexpected token for toolkit/devtools/server/actors/script.js 501:18 error Unexpected token this toolkit/devtools/server/actors/storage.js 888:27 error Unexpected token for toolkit/devtools/server/actors/styleeditor.js 48:16 error Unexpected token this toolkit/devtools/server/actors/styles.js 131:14 error Unexpected token this toolkit/devtools/server/actors/stylesheets.js 52:16 error Unexpected token this toolkit/devtools/server/actors/utils/TabSources.js 338:13 error Unexpected token for toolkit/devtools/server/actors/webapps.js 568:15 error Unexpected identifier toolkit/devtools/server/actors/webbrowser.js 88:10 error Illegal yield expression toolkit/devtools/server/actors/webgl.js 123:21 error Unexpected token this toolkit/devtools/server/main.js 50:20 error Unexpected identifier toolkit/devtools/server/protocol.js 199:45 error Unexpected token for toolkit/devtools/server/tests/mochitest/inspector-helpers.js 128:64 error Unexpected token for toolkit/devtools/server/tests/unit/test_frameactor-04.js 65:11 error Unexpected identifier toolkit/devtools/server/tests/unit/test_frameactor-05.js 60:39 error Unexpected token for toolkit/devtools/server/tests/unit/test_protocol_children.js 32:24 error Unexpected string toolkit/devtools/server/tests/unit/testactors.js 50:38 error Unexpected token for toolkit/devtools/styleinspector/css-logic.js 232:11 error Unexpected identifier toolkit/devtools/transport/stream-utils.js 120:15 error Unexpected token if toolkit/devtools/transport/tests/unit/test_bulk_error.js 18:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/test_client_server_bulk.js 19:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/test_dbgsocket.js 91:13 error Unexpected token if toolkit/devtools/transport/tests/unit/test_delimited_read.js 11:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/test_no_bulk.js 19:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/test_queue.js 18:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/test_transport_bulk.js 14:11 error Unexpected identifier toolkit/devtools/transport/tests/unit/testactors.js 49:38 error Unexpected token for toolkit/devtools/transport/transport.js 277:15 error Unexpected token if toolkit/devtools/webconsole/network-monitor.js 98:15 error Unexpected token if toolkit/devtools/webconsole/test/common.js 205:25 error Illegal yield expression toolkit/devtools/webconsole/utils.js 296:17 error Unexpected token if
Blocks: 887895
This isn't all of them yet, but I've been removing spidermonkey specific js from all over the devtools code for 2 days, I'm sick of it now. If someone wants to take over from here, I'd be thankful. Otherwise, I'll return to this bug in a few days.
I've got it.
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Mostly done, but eslint doesn't like this from script.js: const steppingHookState = { pauseAndRespond: (aFrame, onPacket=(k)=>k) => { return this._pauseAndRespond(aFrame, { type: "resumeLimit" }, onPacket); }, Specifically it errors on the second arrow: toolkit/devtools/server/actors/script.js 897:50 error Unexpected token => Seems like a bug in eslint, I'm filing it there. A workaround seems to be to remove the parens around (k).
Here's the patch.
Attachment #8610115 - Attachment is obsolete: true
Attachment #8611325 - Flags: review?(pbrosset)
Comment on attachment 8611325 [details] [diff] [review] remove SpiderMonkey-specific syntax from toolkit/devtools Review of attachment 8611325 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this looks mainly good to me. I've just picked up a couple of potential problems with replacing foreach loops with for..of or [x for()] array constructions with map() which I fear may fail. But I don't know the code in details enough to be sure. Feel free to ignore me if you already checked. Anyway, I guess only a full try run will tell us if something is wrong or not (make sure you include mochitest-other, xpcshell and mochitest-devtools tests). Marking as R+ because there's not much sense in me checking again after you've corrected these or verified that they were correct in the first place. Just make sure you have a green try build before landing. And thanks for doing this! ::: toolkit/devtools/server/tests/mochitest/inspector-helpers.js @@ +124,5 @@ > let serverWalker = serverConnection.getActor(walker.actorID); > > return { > root: serverOwnershipSubtree(serverWalker, serverWalker.rootDoc ), > + orphaned: serverWalker._orphaned.map(o => serverOwnershipSubtree(serverWalker, o.rawNode)), If I'm not mistaken, the _orphaned property is a Set, not an Array, so map won't work, you could wrap it with [...serverWalker._orphaned] to map on it. ::: toolkit/devtools/styleinspector/css-logic.js @@ +407,5 @@ > * will be the this object when aCallback executes. > */ > forEachSheet: function CssLogic_forEachSheet(aCallback, aScope) > { > + for (let sheets of this._sheets) { Isn't this._sheets an object (I see `this._sheets = {}` in CssLogic_reset). If it is then we need a for..in loop, not a for..of loop. @@ +436,5 @@ > * otherwise false is returned. > */ > forSomeSheets: function CssLogic_forSomeSheets(aCallback, aScope) > { > + for (let sheets of this._sheets) { Same here, but maybe I'm mistaken.
Attachment #8611325 - Flags: review?(pbrosset) → review+
(In reply to Patrick Brosset [:pbrosset] [:patrick] [:pbro] from comment #6) > Just make sure you have a green try build before landing. Yeah, don't worry about that :) > > + for (let sheets of this._sheets) { > > Isn't this._sheets an object (I see `this._sheets = {}` in CssLogic_reset). > If it is then we need a for..in loop, not a for..of loop. I will double check the ones you pointed out. I went through everything by hand and ran the tookit/devtools tests to isolate earlier mistakes along these lines. But it's possible that the tests don't provide coverage in some areas.
Some updates from re-inspecting the changes and running more tests.
Attachment #8611325 - Attachment is obsolete: true
One spot needed to use next to send a value to an es6 generator.
Attachment #8612361 - Attachment is obsolete: true
Attachment #8612455 - Flags: review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: