1305777 - Remove old RDM

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect, P3)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Once we're confident that the RDM redesign is performing well (probably a few cycles after 52?), we should remove the old RDM code.

Notes:

* Firefox OS desktop builds use the old RDM, but this may not matter anymore since it's being removed from the codebase in other ways already
* responsivedesign.jsm currently switches between the two paths, so we'll need to update integration points that call this.  Perhaps leave a small JSM around for Firefox UI code?  Unsure exactly.
* The responsivedesign-client.js frame script is also used by the new RDM, so it should be moved there
* Remove devtools/client/responsivedesign
* Possibly rename devtools/client/responsive.html to devtools/client/responsive assuming it is done in a way that preserves history

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

browser_styleeditor_media_sidebar_links.js was changed in bug 1296498 to support old and new RDM APIs.  The path for old RDM can be removed here.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

New RDM is only used in e10s mode, and old RDM is used as the fallback if e10s is off.  So, we should keep old RDM around until e10s is the default everywhere.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Now that e10s has become the default, it's likely a good time to do this.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

:bgrins notes a few more steps in bug 1403360:

* Remove the devtools.responsive.html.enabled pref
* Update / remove MDN docs - https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode_(before_Firefox_52), and the banner at https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Move RDM frame script to new RDM.

Review commit: https://reviewboard.mozilla.org/r/183782/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183782/

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Remove RDM manager switch and pref.

This removes the switching based on the RDM.html pref and the pref itself.

Some files in old RDM are updated for completeness, but they are about to be
removed anyway.

Review commit: https://reviewboard.mozilla.org/r/183784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183784/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Move RDM GCLI commands to new RDM.

Review commit: https://reviewboard.mozilla.org/r/183786/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183786/

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Remove old RDM.

Review commit: https://reviewboard.mozilla.org/r/183788/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183788/

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Clean up tests that supported both RDMs.

Review commit: https://reviewboard.mozilla.org/r/183790/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183790/

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=295e6a8a77cec0d3a921b2f56c1a18e549d54c45

Comment 12

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912458 [details]
Bug 1305777 - Move RDM frame script to new RDM.

https://reviewboard.mozilla.org/r/183780/#review189200

Thanks, it seems to look good. I tried to manual test everything and haven't seen any glitch.

Should we also remove these files as well?
  devtools/client/themes/responsivedesign.css
  devtools/client/locales/en-US/responsiveUI.properties

And also: devtools/shared/touch/simulator.js and devtools/shared/touch/simulator-content.js
It looks like only devtools/shared/touch/simulator-core.js is used by EmulationActor (which is the way to do touch for responsive.html).

I got this exception, but can't easily reproduce:
Full message: TypeError: this.emulationFront is undefined
Full stack: ResponsiveUI.prototype.updateTouchSimulation<@resource://devtools/shared/base-loader.js -> resource://devtools/client/responsive.html/manager.js:558:7
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
TaskImpl@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:272:3
asyncFunction@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:246:14
onChangeTouchSimulation@resource://devtools/shared/base-loader.js -> resource://devtools/client/responsive.html/manager.js:499:5
handleMessage@resource://devtools/shared/base-loader.js -> resource://devtools/client/responsive.html/manager.js:462:9
handleEvent@resource://devtools/shared/base-loader.js -> resource://devtools/client/responsive.html/manager.js:434:9
EventListener.handleEvent*ResponsiveUI.prototype.init</this.swap<.getInnerBrowser<@resource://devtools/shared/base-loader.js -> resource://devtools/client/responsive.html/manager.js:332:9

It may only happen when shaking the RDM on and off.

Comment 13

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912458 [details]
Bug 1305777 - Move RDM frame script to new RDM.

https://reviewboard.mozilla.org/r/183782/#review189206

Thanks for the multiple patches, it really helped review such work!

Comment 14

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912459 [details]
Bug 1305777 - Remove RDM manager switch and pref.

https://reviewboard.mozilla.org/r/183784/#review189208

Comment 15

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912460 [details]
Bug 1305777 - Move RDM GCLI commands to new RDM.

https://reviewboard.mozilla.org/r/183786/#review189210

Comment 16

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912461 [details]
Bug 1305777 - Remove old RDM.

https://reviewboard.mozilla.org/r/183788/#review189212

Comment 17

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912462 [details]
Bug 1305777 - Clean up tests that supported both RDMs.

https://reviewboard.mozilla.org/r/183790/#review189214

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alexandre Poirot [:ochameau] from comment #12)
> Thanks, it seems to look good. I tried to manual test everything and haven't
> seen any glitch.

Thanks for all the reviews!

> Should we also remove these files as well?
>   devtools/client/themes/responsivedesign.css
>   devtools/client/locales/en-US/responsiveUI.properties

Yes, thanks for catching that.  I also found:

* bits of CSS in browser.css
* images for the old UI

Guess I went a little too fast! :/ I'll remove them too.

> And also: devtools/shared/touch/simulator.js and
> devtools/shared/touch/simulator-content.js
> It looks like only devtools/shared/touch/simulator-core.js is used by
> EmulationActor (which is the way to do touch for responsive.html).

Another good catch!  I'll post an extra patch for this part.

> I got this exception, but can't easily reproduce:
> Full message: TypeError: this.emulationFront is undefined
> Full stack:
> ResponsiveUI.prototype.updateTouchSimulation<@resource://devtools/shared/
> base-loader.js -> resource://devtools/client/responsive.html/manager.js:558:7
> _run@resource://devtools/shared/base-loader.js ->
> resource://devtools/shared/task.js:310:39
> TaskImpl@resource://devtools/shared/base-loader.js ->
> resource://devtools/shared/task.js:272:3
> asyncFunction@resource://devtools/shared/base-loader.js ->
> resource://devtools/shared/task.js:246:14
> onChangeTouchSimulation@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/responsive.html/manager.js:499:5
> handleMessage@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/responsive.html/manager.js:462:9
> handleEvent@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/responsive.html/manager.js:434:9
> EventListener.handleEvent*ResponsiveUI.prototype.init</this.swap<.
> getInnerBrowser<@resource://devtools/shared/base-loader.js ->
> resource://devtools/client/responsive.html/manager.js:332:9
> 
> It may only happen when shaking the RDM on and off.

Hmm, it doesn't seem related to the removing old RDM at least...  Maybe if you toggle while also toggling RDM itself quickly?  If we find a good STR, let's file a separate bug.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Clean up touch simulator after old RDM removal.

Only `simulator-core` is used by new RDM via the Emulation actor, so I've moved
this server side and renamed it to `touch-simulator`.

Review commit: https://reviewboard.mozilla.org/r/184264/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184264/

Comment 20

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912461 [details]
Bug 1305777 - Remove old RDM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183788/diff/1-2/

Comment 21

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912462 [details]
Bug 1305777 - Clean up tests that supported both RDMs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183790/diff/1-2/

Comment 22

[Alexandre Poirot [:ochameau]]

Comment on attachment 8912940 [details]
Bug 1305777 - Clean up touch simulator after old RDM removal.

https://reviewboard.mozilla.org/r/184264/#review189432

devtools/server/touch-simulator.js isn't declared as a copy of simulator-core.js?
Are you using cinnabar? If yes, you need glandium's magick trick...

Otherwise, /devtools/server is quite messy. We tend to just drop things either in server/ or actors/utils/ folders.
What about putting touch-simulator in devtools/server/actors/emulation/touch-simulator? It helps following that's just a emulation actor dependency.

Comment 23

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912940 [details]
Bug 1305777 - Clean up touch simulator after old RDM removal.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184264/diff/1-2/

Comment 24

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Alexandre Poirot [:ochameau] from comment #22)
> devtools/server/touch-simulator.js isn't declared as a copy of
> simulator-core.js?
> Are you using cinnabar? If yes, you need glandium's magick trick...

Thanks for the tip, seems like it would be better as a Git config option to avoid editing cinnabar...

> Otherwise, /devtools/server is quite messy. We tend to just drop things
> either in server/ or actors/utils/ folders.
> What about putting touch-simulator in
> devtools/server/actors/emulation/touch-simulator? It helps following that's
> just a emulation actor dependency.

I agree, moved!

Comment 25

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1305777 - Fix ESLint issues for moved files.

Review commit: https://reviewboard.mozilla.org/r/184662/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184662/

Comment 26

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=31bd73af6db36ef2f336cd2cdfb226b9ff1f635c

Comment 27

[Julian Descottes [:jdescottes]]

Comment on attachment 8913292 [details]
Bug 1305777 - Fix ESLint issues for moved files.

https://reviewboard.mozilla.org/r/184662/#review189838

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a0227d048e8
Move RDM frame script to new RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/6ae05de7437d
Remove RDM manager switch and pref. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/776ad30a20db
Move RDM GCLI commands to new RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/6e0cb1927bfe
Remove old RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/c311ac4b05b7
Clean up tests that supported both RDMs. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/0169ee5c78fb
Clean up touch simulator after old RDM removal. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/ffb4e22a4347
Fix ESLint issues for moved files. r=jdescottes

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing devtools' devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js on Windows 7 debug without e10s:

https://hg.mozilla.org/integration/autoland/rev/1fe25bc278e75281a56b0e400f55a0f73bc5635e
https://hg.mozilla.org/integration/autoland/rev/883484b7794765b796e9eeeef38a958b7a6052ae
https://hg.mozilla.org/integration/autoland/rev/4e494ae5b19d9280c2c228f5340fd5f2d405cc0a
https://hg.mozilla.org/integration/autoland/rev/27bf9dc1839e5feb9bb59c1c554819319e39aa49
https://hg.mozilla.org/integration/autoland/rev/d9d93f156d3b3afe7a70e6b867e503c653586107
https://hg.mozilla.org/integration/autoland/rev/3ff120562b4a06a3eb829901771e02d1f8168fd2
https://hg.mozilla.org/integration/autoland/rev/84606fe1cd2df32342f4d3eb512bf911e2151cd0

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ffb4e22a4347adfd47536ccec04d3b2e95c8f2c7&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=133983113&repo=autoland

04:15:49     INFO -  697 INFO Testing if media rules have the appropriate number of links
04:15:49     INFO -  698 INFO TEST-PASS | devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js | There should be no links in the first media rule. -
04:15:49     INFO -  699 INFO TEST-PASS | devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js | There should be no links in the second media rule. -
04:15:49     INFO -  700 INFO TEST-PASS | devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js | There should be 1 responsive mode link in the media rule -
04:15:49     INFO -  701 INFO TEST-PASS | devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js | There should be 2 responsive mode links in the media rule -
04:15:49     INFO -  702 INFO Waiting for event: 'media-list-changed' on [object Object].
04:15:49     INFO -  703 INFO Launching responsive mode
04:15:49     INFO -  704 INFO Waiting for content-resize to a width of 400
04:15:49     INFO -  Buffered messages finished
04:15:49    ERROR -  705 INFO TEST-UNEXPECTED-FAIL | devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js:65 - TypeError: rdmUI is undefined
04:15:49     INFO -  Stack trace:
04:15:49     INFO -      testMediaLink@chrome://mochitests/content/browser/devtools/client/styleeditor/test/browser_styleeditor_media_sidebar_links.js:65:3
04:15:49     INFO -      process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
04:15:49     INFO -      walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
04:15:49     INFO -      Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
04:15:49     INFO -      schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
04:15:49     INFO -      completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:315:13
04:15:49     INFO -      promise callback*_handleResultValue@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:386:7
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:318:13
04:15:49     INFO -      process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
04:15:49     INFO -      walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
04:15:49     INFO -      Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
04:15:49     INFO -      schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
04:15:49     INFO -      completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
04:15:49     INFO -      onPacket/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1356:9
04:15:49     INFO -      DevTools RDP*request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1296:14
04:15:49     INFO -      generateRequestMethods/</frontProto[name]@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1454:14
04:15:49     INFO -      PerformanceFront<.connect<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/fronts/performance.js:34:28
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
04:15:49     INFO -      process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
04:15:49     INFO -      walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
04:15:49     INFO -      Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
04:15:49     INFO -      schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
04:15:49     INFO -      completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:315:13
04:15:49     INFO -      promise callback*_handleResultValue@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:386:7
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:318:13
04:15:49     INFO -      promise callback*_handleResultValue@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:386:7
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:318:13
04:15:49     INFO -      TaskImpl@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:272:3
04:15:49     INFO -      asyncFunction@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:246:14
04:15:49     INFO -      StyleEditorUI.prototype.initialize<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:144:11
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
04:15:49     INFO -      process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
04:15:49     INFO -      walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
04:15:49     INFO -      Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
04:15:49     INFO -      schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
04:15:49     INFO -      completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
04:15:49     INFO -      onPacket/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1356:9
04:15:49     INFO -      DevTools RDP*request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1296:14
04:15:49     INFO -      generateRequestMethods/</frontProto[name]@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1454:14
04:15:49     INFO -      StyleEditorUI.prototype.initialize<@resource://devtools/client/styleeditor/StyleEditorUI.jsm:143:29
04:15:49     INFO -      _run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
04:15:49     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
04:15:49     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9
04:15:49     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59
04:15:49     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:798:9
04:15:49     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:697:9
04:15:49     INFO -      SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59

Comment 31

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912458 [details]
Bug 1305777 - Move RDM frame script to new RDM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183782/diff/1-2/

Comment 32

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912459 [details]
Bug 1305777 - Remove RDM manager switch and pref.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183784/diff/1-2/

Comment 33

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912460 [details]
Bug 1305777 - Move RDM GCLI commands to new RDM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183786/diff/1-2/

Comment 34

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912461 [details]
Bug 1305777 - Remove old RDM.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183788/diff/2-3/

Comment 35

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912462 [details]
Bug 1305777 - Clean up tests that supported both RDMs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183790/diff/2-3/

Comment 36

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8912940 [details]
Bug 1305777 - Clean up touch simulator after old RDM removal.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184264/diff/2-3/

Comment 37

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8913292 [details]
Bug 1305777 - Fix ESLint issues for moved files.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184662/diff/1-2/

Comment 38

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e8bdcf976ecf5612f83da6c1ac74db2e1253058

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc55e32ba087
Move RDM frame script to new RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/71faac859b4a
Remove RDM manager switch and pref. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/3c1ab86a8611
Move RDM GCLI commands to new RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/d939f110bf2e
Remove old RDM. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/a2c2643cf909
Clean up tests that supported both RDMs. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/e2ea4cf82cae
Clean up touch simulator after old RDM removal. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/3b5b98033433
Fix ESLint issues for moved files. r=jdescottes

Comment 40

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fc55e32ba087
https://hg.mozilla.org/mozilla-central/rev/71faac859b4a
https://hg.mozilla.org/mozilla-central/rev/3c1ab86a8611
https://hg.mozilla.org/mozilla-central/rev/d939f110bf2e
https://hg.mozilla.org/mozilla-central/rev/a2c2643cf909
https://hg.mozilla.org/mozilla-central/rev/e2ea4cf82cae
https://hg.mozilla.org/mozilla-central/rev/3b5b98033433

Comment 41

[Oriol Brufau [:Oriol]]

If you remove the old RDM you could at least make the new one support non-e10s.

Comment 42

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Oriol Brufau [:Oriol] from comment #41)
> If you remove the old RDM you could at least make the new one support
> non-e10s.

I am not aware of strong need for this.  e10s is now enabled everywhere by default (minus possibly a11y for now? but it's on the way).  This is exactly why we waited until now before removing the old RDM.

If you have a use case in mind where non-e10s is needed with new RDM, please file a new bug and we can discuss there.

Comment 43

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Marking "dev-doc-needed" to consider removing the docs about the pre-52 version:

https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode_(before_Firefox_52)

and remove the link to that page from the new docs:

https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode

Comment 44

[Chris Mills [:cmills]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #43)
> Marking "dev-doc-needed" to consider removing the docs about the pre-52
> version:
> 
> https://developer.mozilla.org/en-US/docs/Tools/
> Responsive_Design_Mode_(before_Firefox_52)
> 
> and remove the link to that page from the new docs:
> 
> https://developer.mozilla.org/en-US/docs/Tools/Responsive_Design_Mode

Done. 

I've also added a note to the Fx58 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/58#Developer_Tools