1386812 - Check toolkit rather than OS under devtools/

Status: NEW

(DevTools :: General, enhancement, P3)

Description

[Jan Beich]

When checking Services.appinfo.OS to handle OS_TARGET=Linux other Tier3 platforms (e.g. BSDs, Solaris) using Gtk are often overlooked.

$ rg -lt js -g '!*/test*' --sort-files '"Linux"'
addon-sdk/source/lib/sdk/system/child_process/subprocess.js
browser/extensions/mortar/host/flash/bootstrap.js
browser/extensions/mortar/host/pdf/bootstrap.js
devtools/client/debugger/new/integration-tests.js
devtools/client/inspector/local-toolbox.js
devtools/client/netmonitor/index.js
devtools/client/shared/developer-toolbar.js
devtools/client/webconsole/local-dev/index.js
devtools/client/webide/modules/addons.js
devtools/client/webide/modules/simulator-process.js
devtools/shared/system.js
js/src/octane/mandreel.js
testing/marionette/atom.js

Comment 1

[Jan Beich]

Attachment: Bug 1386812 - Apply Linux theme on Tier3 platforms under devtools/.

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

Comment 2

[Jan Beich]

Comment on attachment 8893088 [details]
Bug 1386812 - Apply Linux theme on Tier3 platforms under devtools/.

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

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8893088 [details]
Bug 1386812 - Apply Linux theme on Tier3 platforms under devtools/.

https://reviewboard.mozilla.org/r/164090/#review169738

Thank you for the patch.  I didn't see an r? in the commit message, so I figured I would just drop in and review it.

This patch looks reasonable to me.

I think it would be good to at least file a bug against devtools-core: https://github.com/devtools-html/devtools-core/issues.
That code has a Services.appinfo shim: https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-modules/src/Services.js#L517,
which perhaps should be updated.  "Perhaps" - because it's not clear to me whether the shim is in active use.