* code tidy up: always evaluates
* tidy up: is always true
* tidy up: remove unused code
* always true/false variables
* unused variable
* tidy up: remove unused code in caretPosition.js
* for squash: Revert "tidy up: remove unused code in caretPosition.js"
The `if` condition was previously always true, so the body should be
preserved. If the body is preserved, other logic can be deleted. I
opened PR #4845 to clean it all up.
This reverts commit 75b03e5a7dc1ff9a8728ed2341fd9fe970d0615f.
* for squash: simplify
* for squash: Explain that the getter is used for its side effects
It's very weird to call a getter without using its return value. Add a
comment explaining why this is done so that the reader doesn't get
confused.
* for squash: Revert "tidy up: remove unused code"
The exception test was the purpose of the code.
This reverts commit 85153b167613b2513fff99e22b8ded8ea1e4547b.
* for squash: Log the tsort results
Co-authored-by: Richard Hansen <rhansen@rhansen.org>
Before this change, the `author` attribute was silently discarded
during `.map()` iteration and the name of the attribute to remove was
included twice with two different values.
Before this commit, the callback passed to `.map()` during attribute
removal was a normal function, not an arrow function. This meant that
the value of `this` in the function body depended on how the callback
was invoked. In this case, the callback was invoked without any
explicit context (it was not called as a method, nor was it called via
`.call()`, `.apply()`, or `.bind()`). Without any explicit context,
the value of `this` depends on strict mode. Currently the function is
in sloppy mode, so `this` refers to the "global this" object (a.k.a.,
`window`). It doesn't make sense for the callback to reference
`window.author`, so I'm assuming the previous behavior was a bug.
Now the function is an arrow function, so the value of `this` comes
from the enclosing lexical context, which in this case is the
AttributeManager object. I believe that was the original intention.
Normally I would let `eslint --fix` do this for me, but there's a bug
that causes:
const x = function ()
{
// ...
};
to become:
const x = ()
=> {
// ...
};
which ESLint thinks is a syntax error. (It probably is; I don't know
enough about the automatic semicolon insertion rules to be confident.)
When comparing original content with the changes made by the user, we
need to ignore some line attribs that are added by content collector,
otherwise we would consider the change started on the first char of the
line -- the '*' that is added when line has line attribs.
In order to be able to handle both #3354 and #3118, we need to take into
account both the styles attribs (to fix#3354) and the line attribs
defined by any of the plugins (to fix#3118), but we can ignore those
extra line attribs that are added by Etherpad and do not add any
functionality (`'lmkr', 'insertorder', 'start'`).
This avoids raising error 'Trying to submit changes as another author in
changeset' when 2 authors change line attributes of the same line. This
fixes issue #2925.
