Skip to content

fix-polyfill WebRTC-events-when-proto-is-non-writable-but-configurable#1194

Open
johnny-quesada-developer wants to merge 1 commit into
webrtcHacks:mainfrom
johnny-quesada-developer:fix-polyfill-WebRTC-events-when-proto-is-non-writable-but-configurable
Open

fix-polyfill WebRTC-events-when-proto-is-non-writable-but-configurable#1194
johnny-quesada-developer wants to merge 1 commit into
webrtcHacks:mainfrom
johnny-quesada-developer:fix-polyfill-WebRTC-events-when-proto-is-non-writable-but-configurable

Conversation

@johnny-quesada-developer

@johnny-quesada-developer johnny-quesada-developer commented Jun 3, 2026

Copy link
Copy Markdown

Summary

Fix wrapPeerConnectionEvent so it does not skip event polyfilling when RTCPeerConnection.prototype event listener methods are non-writable but still configurable through Object.defineProperty.

Background

I found this issue while testing a WebRTC application embedded in Wix.

The current implementation checks the descriptor of EventTarget.prototype.addEventListener, but the polyfill actually patches methods on RTCPeerConnection.prototype:

proto.addEventListener = function(nativeEventName, cb) { ... };
proto.removeEventListener = function(nativeEventName, cb) { ... };

In some browsers or environments, these properties can behave as read-only, so direct assignment throws a TypeError:

Cannot assign to read only property 'addEventListener' of object '#<RTCPeerConnection>'

The latest adapter version avoids the crash by checking whether EventTarget.prototype.addEventListener is writable before applying the polyfill. However, that check is not enough to determine whether RTCPeerConnection.prototype can actually be patched.

This is a known issue that has been reported in other contexts as well:

Reproduction

This can be reproduced with this Wix test page:

By default, the page loads with the polyfill fix applied. To disable the fix and reproduce the original error, run this in the browser console:

localStorage.setItem('error-test', true);

Then reload the page. You will see the crash in the console.

Important: Because the test page uses an old version that does not have the EventTarget.prototype descriptor check, the issue was loud and visible. The latest version of adapter does not crash, but it also does not apply the polyfill.

A minimal code example to illustrate the underlying JavaScript behavior:

(function () {
  'use strict';

  const object = {};

  Object.defineProperty(object, 'readonly', {
    value: false,
    writable: false,
    configurable: true,
  });

  // This throws because the property is read-only.
  object.readonly = someNewValue();
})();

However, even when a property is non-writable, it can still be redefined:

Object.defineProperty(object, 'readonly', {
  value: someNewValue(),
  writable: true,
  configurable: true,
});

Root Cause

The old guard checked EventTarget.prototype.addEventListener.writable globally:

const addEventListener = Object.getOwnPropertyDescriptor(
  EventTarget.prototype,
  'addEventListener'
);

if (!addEventListener.writable) {
  return;
}

The main issue is that the EventTarget.prototype.addEventListener is not writeable, cannot be assigned... but the target RTCPeerConnection.prototype can still define its own addEventListener

Fix

Introduce tryMakePropertyWritable, which attempts to redefine the target property as writable before applying the polyfill.

Instead of relying on direct assignment, it uses Object.defineProperty:

Object.defineProperty(target, property, {
  value: target[property],
  writable: true,
  configurable: true,
});

If the property can be redefined, the helper returns true. If the property is truly locked, it catches the error and returns false.

The polyfill now only proceeds when both required methods can be prepared:

  const isAddEventListenerWritable = tryMakePropertyWritable(proto, 'addEventListener');
  const isRemoveEventListenerWritable = tryMakePropertyWritable(proto, 'removeEventListener');
  const canPolyfill = isAddEventListenerWritable && isRemoveEventListenerWritable;

  if (!canPolyfill) {
    log('Unable to polyfill events');
    return;
  }

This makes the check more accurate because it validates the actual prototype being patched.

Behavior Before

const addEventListener = Object.getOwnPropertyDescriptor(
  EventTarget.prototype,
  'addEventListener'
);

if (!addEventListener.writable) {
  return;
}

The polyfill could be skipped based on the descriptor of EventTarget.prototype, even though the actual patch target was RTCPeerConnection.prototype.

Behavior After

const proto = window.RTCPeerConnection.prototype;

const canPolyfill =
  tryMakePropertyWritable(proto, 'addEventListener') &&
  tryMakePropertyWritable(proto, 'removeEventListener');

if (!canPolyfill) {
  log('Unable to polyfill events');
  return;
}

The polyfill now checks whether the actual target can be patched before replacing the event listener methods.

Test Plan

Added unit tests for wrapPeerConnectionEvent covering:

  • Configurable properties

    • addEventListener and removeEventListener can be redefined.
    • The polyfill is applied successfully.
    • Events are transformed before reaching the listener.
  • Non-configurable properties

    • The polyfill does not throw.
    • The prototype is left untouched.
    • Native callbacks continue to work without wrapping.

@fippo

fippo commented Jun 7, 2026

Copy link
Copy Markdown
Member

so #1177 again... but the only usage affected should be shimSendThrowTypeError which is on its way out in M149?

@johnny-quesada-developer

Copy link
Copy Markdown
Author

so #1177 again... but the only usage affected should be shimSendThrowTypeError which is on its way out in M149?

Hi @fippo,

There are two other functions that also call wrapPeerConnectionEvent:

  • shimRTCIceCandidate
  • shimRTCIceCandidateRelayProtocol

Thanks for checking on this.

@fippo

fippo commented Jun 22, 2026

Copy link
Copy Markdown
Member

but the first should be inert due to

  if (!window.RTCIceCandidate || (window.RTCIceCandidate && 'foundation' in
      window.RTCIceCandidate.prototype)) {
    return;
  }

in all browsers (since 2018 for Chrome). Firefox still seems to need the second shim but that is not polyfillable so inert too.

Safari... hey @youennf, would you mind reducing the need for adapter? Gets you two points here

@fippo

fippo commented Jun 25, 2026

Copy link
Copy Markdown
Member

Does checking RTCPeerConnection.prototype instead of EventTarget.prototype here improve the situation?

@johnny-quesada-developer

johnny-quesada-developer commented Jun 29, 2026

Copy link
Copy Markdown
Author

RTCPeerConnection

Hi @fippo, no, you cannot get the addEventListener descriptor from RTCPeerConnection because that property is not in the RTCPeerConnection prototype, is inherit from EventTarget.

image

Direct assignment to RTCPeerConnection.prototype.addEventListener will fail because the inherited property is non-writable, while Object.defineProperty can still create a new own property on RTCPeerConnection.prototype with writeable and configurable as true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants