Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const {template} = require('ep_plugin_helpers');

const eejs = require('ep_etherpad-lite/node/eejs/');
const eejs = require('ep_etherpad-lite/node/eejs');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. No regression test for eejs require 📘 Rule violation ☼ Reliability

This PR changes the eejs require path as a bug fix/compatibility fix but does not add or update
any automated regression test that would fail before the change and pass after it. This increases
the risk of reintroducing the trailing-slash issue or missing related resolution regressions.
Agent Prompt
## Issue description
A bug fix was merged without an accompanying regression test.

## Issue Context
The PR fixes ESM compatibility by removing the trailing slash from the `eejs` require path, but no automated test was added/updated to prevent regressions.

## Fix Focus Areas
- index.js[3-6]
- package.json[25-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

const settings = require('ep_etherpad-lite/node/utils/Settings');

exports.loadSettings = (hookName, context, cb) => {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"description": "Adds inline toolbar support to a Pad",
"name": "ep_inline_toolbar",
"version": "0.2.57",
"version": "0.2.59",
"author": {
"name": "John McLear",
"email": "john@mclear.co.uk"
Expand Down
50 changes: 37 additions & 13 deletions static/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@ const iT = {
};

exports.aceSelectionChanged = (hook, context) => {
const selStart = context.rep.selStart;
const selEnd = context.rep.selEnd;
// Guard for timeslider playback: broadcasted revisions can fire
// selection-change events with a partially-initialized rep. Without
// this guard, accessing rep.selStart[0] throws "Cannot read properties
// of undefined" and surfaces as an error gritter popup (#5214 test).
const rep = context && context.rep;
if (!rep || !rep.selStart || !rep.selEnd) return;
const {selStart, selEnd} = rep;
if ((selStart[0] !== selEnd[0]) || (selStart[1] !== selEnd[1])) {
iT.show();
} else {
Expand All @@ -31,7 +36,13 @@ exports.postAceInit = (hookName, context) => {
});
padInner.on('mouseup', (e) => {
const toolbar = padOuter.find('#inline_toolbar');
const left = e.pageX + padOuter.find('iframe').offset().left;
// If #inline_toolbar wasn't rendered (e.g. timeslider context, which
// doesn't include the eejsBlock_body template), or the ace_outer
// iframe hasn't laid out yet, skip the positioning to avoid a
// .offset().left throw.
const iframeOffset = padOuter.find('iframe').offset();
if (!toolbar.length || !iframeOffset) return;
const left = e.pageX + iframeOffset.left;
toolbar.css({
position: 'absolute',
left,
Expand All @@ -42,11 +53,19 @@ exports.postAceInit = (hookName, context) => {

// Creates the buttons based on settings and draws them hidden on the screen
exports.postToolbarInit = (hook, context) => {
// #inline_toolbar lives in eejsBlock_body, which is only rendered for
// the regular pad layout. In timeslider context the element doesn't
// exist; everything below this guard would either no-op or throw on
// null contentDocument / missing iframe.
if (!$('#inline_toolbar').length) return;

let buttonsToShow = [];
if (clientVars.ep_inline_toolbar && clientVars.ep_inline_toolbar.length) {
buttonsToShow = clientVars.ep_inline_toolbar[0];
} else {
} else if (clientVars.ep_inline_toolbar && clientVars.ep_inline_toolbar.left) {
buttonsToShow = clientVars.ep_inline_toolbar.left.flat();
} else {
return; // unrecognized settings shape — nothing to render
}
$('#editbar .menu_left').children('[data-key]').each(function () {
const key = $(this).data('key');
Expand Down Expand Up @@ -74,13 +93,18 @@ exports.postToolbarInit = (hook, context) => {
// toolbar colours). Using a stylesheet rather than an inline style lets
// the browser's prefers-color-scheme media query do the work, so the
// toolbar automatically adapts without any JS re-evaluation.
const outerDoc = $('iframe[name="ace_outer"]')[0].contentDocument;
const $style = $('<style>').text(
'#inline_toolbar { background-color: #f4f4f4; }\n' +
'@media (prefers-color-scheme: dark) {\n' +
' #inline_toolbar { background-color: #1a1a1a; }\n' +
'}',
);
$(outerDoc.head).append($style);
$('#inline_toolbar').detach().appendTo(padOuter[0]);
const aceOuter = $('iframe[name="ace_outer"]')[0];
const outerDoc = aceOuter && aceOuter.contentDocument;
if (outerDoc && outerDoc.head) {
const $style = $('<style>').text(
'#inline_toolbar { background-color: #f4f4f4; }\n' +
'@media (prefers-color-scheme: dark) {\n' +
' #inline_toolbar { background-color: #1a1a1a; }\n' +
'}',
);
$(outerDoc.head).append($style);
}
if (padOuter.length) {
$('#inline_toolbar').detach().appendTo(padOuter[0]);
}
};
Loading