diegodlh / zotero-cita

Cita: a Wikidata addon for Zotero with citations metadata support
GNU General Public License v3.0
233 stars 12 forks source link

Simplify react-dom patch #294

Closed diegodlh closed 1 month ago

diegodlh commented 1 month ago

Commit 6e2018e fixed the PID row text input bug where we were getting activeElement.detachEvent is not a function errors when clicking on text inputs in PID rows.

The fix involved importing createRoot dynamically. Otherwise, if imported at the top of the file, because window would be undefined, canUseDOM would be false in react-dom's react-dom.js, which in turn makes isInputEventSupported be false, which then makes handleEventsForInputEventPolyfill be called, which in the end makes activeElement.attachEvent() and activeElement.detachEvent() be called, which are both undefined.

However, as a result of importing createRoot dynamically, now canUseDOM is true, and the following line in react-dom-js is run:

if (navigator.userAgent.indexOf('Chrome') > -1 && navigator.userAgent.indexOf('Edge') === -1 || navigator.userAgent.indexOf('Firefox') > -1) {

However, navigator is not defined and an error is thrown. This is why a patch to react-dom was applied in f1792e7. I think a simpler patch could be applied that simply points navigator to window.navigator if undefined.

Anyways, I feel this whole fix is a little difficult to follow. Apparently a more straightforward fix may be possible (based on capricorn86/happy-dom/issues/534) which simply patches react-dom by replacing activeElement.attachEvent() and activeElement.detachEvent() calls with activeElement.addEventListener() and activeElement.removeEventListener() calls, respectively.

I have tested it and it seems to work. But because I'm not sure what the "PID row text input bug" before the original fix implied exactly, I would appreciate @Dominic-DallOsto's comments before merging.

github-actions[bot] commented 1 month ago

:rocket: This ticket has been resolved in v1.0.0-beta.1. See Release 1.0.0-beta.1 for release notes.