Name colours (e.g. "red", "blue") no longer work in 5.4.0 in browser if global `process` variable is defined

[krassowski]

Details

• Browser and browser version: all
• xterm.js version: 5.4.0

Steps to reproduce

1. Define a terminal with a theme like:

   const term = new Terminal({theme: {
   background: 'black',
   foreground: 'blue'
   }});

2. Try to run it in a webpack served application with a process fallback such as:

   module.exports = {
   resolve: {
   fallback: {
     process: require.resolve('process/browser')
   }
   }
   }

Se no error, and no warning but named colours no longer work.

This was introduced by https://github.com/xtermjs/xterm.js/pull/4851 which changed heuristic for detecting whether xterm.js is running in a browser or in a Node,js environment. It now simply looks for whether a process variable is defined, which makes isNode incorrectly true in many browser environments. These means that ctx is never defined here:

https://github.com/xtermjs/xterm.js/blob/0658719d23642ed70f026003aeff60bf05c31358/src/common/Color.ts#L119-L134

and then toColor short-circiuts due to lack of $ctx here:

https://github.com/xtermjs/xterm.js/blob/0658719d23642ed70f026003aeff60bf05c31358/src/common/Color.ts#L184-L187

Note that presence of process in webpack-served browser apps is a common concern - see the highly up-voted first comment on: https://stackoverflow.com/a/34550964/6646912

I found that many proposed solutions stop working if the code has been webpack'd, because webpack defines all kinds of process, module, require, etc symbols. window usually still works, unless of course you additionally also want to detect being run from a webworker, where there is no window.

Now process/browser shim is actually easy to detect because it has a .browser attribute and it's title is "browser". So, checking for: process.title !== 'browser' && !process.browser would already solve this for me.

However, something to consider is whether rather than checking for isNode in:

https://github.com/xtermjs/xterm.js/blob/0658719d23642ed70f026003aeff60bf05c31358/src/common/Color.ts#L122-L129

would it be better to just try-catch here? It (maybe naively) appears to me that what matters is not the fact of running on Node or not, but the fact of whether canvas context can be obtained or not.

What do you think is the best way forward?

[krassowski]

CC @davidfiala as author of https://github.com/xtermjs/xterm.js/pull/4851. What do you think about checking process.title? On node that would be equal to "node". process.browser on node would be undefined.

[krassowski]

@Tyriar thank you very much for a fix! Is there a chance that it would get released as 5.4.1? If not, is 5.5.0 following a fix-calendar schedule or will it be released soon after all things planned are done (I do not see anything open tagged for 5.5.0: https://github.com/xtermjs/xterm.js/issues?q=is%3Aopen+is%3Aissue+milestone%3A5.5.0)

[Tyriar]

@krassowski we've been more flexible (lazy?) about releasing recently, but you asked on the right week so I can put together a release today or tomorrow. Since there are very few changes it should be very easy to put together.