[Slider] Sends incorrect event.target on touch event

[tech-meppem]

Having a slider in a Swipeable Drawer throws "Failed to execute 'contains' on 'Node'", on mobile device and chrome mobile view.

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When attempting to move the slider on mobile view, an error is thrown.

The error is in SwipeableDrawer.js, line 391:
if (open && !backdropRef.current.contains(event.target) && !paperRef.current.contains(event.target)) {

The problem is that event.target is not type 'Node', and is instead an object, like the following:

{
  name: undefined,
  value: 80
}

This only happens if a function has been passed to the onChange prop on the Slider component.

Expected Behavior 🤔

No error should be thrown, and event.target would be a node.

Steps to Reproduce 🕹

Forked from the "Swipeable Edge Drawer" demo:
https://codesandbox.io/s/swipeableedgedrawer-material-demo-forked-kxlvk?file=/demo.tsx
must be on using chrome dev tools, with the mobile view active (device toolbar), otherwise no error happens

Make sure to enable mobile view in chrome dev tools:

Context 🔦

Have a slider in the SwipeableDrawer Edge.

Your Environment 🌎

System:
    OS: Windows 10 10.0.18363
    CPU: (12) x64 AMD Ryzen 5 2600X Six-Core Processor
    Memory: 2.42 GB / 15.93 GB
  Binaries:
    Node: 14.13.1 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.8 - C:\Program Files\nodejs\npm.CMD
  Managers:
    Composer: 1.8.6 - C:\ProgramData\ComposerSetup\bin\composer.BAT
    pip2: 20.2.2 - C:\Program Files (x86)\Python27-18\Scripts\pip2.EXE
    pip3: 19.0.3 - C:\Program Files (x86)\Python37-32\Scripts\pip3.EXE
  Utilities:
    Git: 2.27.0.
  SDKs:
    Windows SDK:
      AllowDevelopmentWithoutDevLicense: Enabled
      AllowAllTrustedApps: Enabled
      Versions: 10.0.17763.0, 10.0.18362.0, 10.0.19041.0
  IDEs:
    VSCode: 1.52.1 - C:\Users\Netduma\AppData\Local\Programs\Microsoft VS Code\bin\code.CMD
    Visual Studio: 16.7.30621.155 (Visual Studio Community 2019)
  Languages:
    Bash: 4.4.20 - C:\Windows\system32\bash.EXE
    PHP: 7.3.7 - C:\PHP7\php.EXE
    Python: 2.7.18
  Browsers:
    Chrome: 88.0.4324.104
    Edge: Spartan (44.18362.449.0)
    Internet Explorer: 11.0.18362.1

[oliviertassinari]

@tech-meppem Interesting, it fails on this line:

https://github.com/mui-org/material-ui/blob/e7bf469195f13bd0bf357111f3e50ac461d2ed38/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js#L444

The mortified event leaks to its parent.

One way to solve the issue is:

diff --git a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.
js
index d47a761883..f05d761d82 100644
--- a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
+++ b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
@@ -232,13 +232,11 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
     ((event, value) => {
       if (!(event instanceof Event)) event.persist();

-      // Redefine target to allow name and value to be read.
+      // Apply a side effect to allow name and value to be read.
       // This allows seamless integration with the most popular form libraries.
       // https://github.com/mui-org/material-ui/issues/13485#issuecomment-676048492
-      Object.defineProperty(event, 'target', {
-        writable: true,
-        value: { value, name },
-      });
+      event.target.name = name;
+      event.target.value = value;

       onChange(event, value);
     });
diff --git a/packages/material-ui/src/Slider/Slider.test.js b/packages/material-ui/src/Slider/Slider.test.js
index 548350c853..11661a1a28 100644
--- a/packages/material-ui/src/Slider/Slider.test.js
+++ b/packages/material-ui/src/Slider/Slider.test.js
@@ -928,7 +928,10 @@ describe('<Slider />', () => {
   });

   it('should pass "name" and "value" as part of the event.target for onChange', () => {
-    const handleChange = stub().callsFake((event) => event.target);
+    const handleChange = stub().callsFake((event) => ({
+      value: event.target.value,
+      name: event.target.name,
+    }));
     const { getByRole } = render(
       <Slider onChange={handleChange} name="change-testing" value={3} />,
     );
@@ -946,7 +949,7 @@ describe('<Slider />', () => {
     expect(handleChange.callCount).to.equal(1);
     expect(handleChange.firstCall.returnValue).to.deep.equal({
       name: 'change-testing',
-      value: 4,
+      value: '4',
     });
   });

Which was first discussed in #22548 (comment).

Another solution is to clone the event to avoid the leak:

diff --git a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.
js
index d47a761883..8304402710 100644
--- a/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
+++ b/packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
@@ -235,12 +235,21 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
       // Redefine target to allow name and value to be read.
       // This allows seamless integration with the most popular form libraries.
       // https://github.com/mui-org/material-ui/issues/13485#issuecomment-676048492
-      Object.defineProperty(event, 'target', {
+      let clonedEvent;
+      // Clone the event if a native event to avoid leaks.
+      // No need to worry about it for SyntheticEvent as they are systematically cloned.
+      if (event instanceof Event) {
+        clonedEvent = new event.constructor(event.type, event);
+      } else {
+        clonedEvent = event;
+      }
+
+      Object.defineProperty(clonedEvent, 'target', {
         writable: true,
         value: { value, name },
       });

-      onChange(event, value);
+      onChange(clonedEvent, value);
     });

   const range = Array.isArray(valueDerived);

Maybe the latter option. The first wouldn't work with Preact as it makes event.target readonly.