| mlaurent | |
| dfaure |
| Frameworks |
If my cursor is on a bullet list, and I press Enter to create a new element,
it registers as 5-7 actions in an undo stack, mostly margins adjustments. So to
rewind it, user has to press Ctrl+Z 5+ times, which is quite frustrating.
Instead this patch suggests to treat _any_ change that is a result of a single
key press as a single event, so user has to press Ctrl+Z just once to undo it
BUG: 256001
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
I tested this with the krichtexteditor test app from kxmlgui, and it works. (It's never too late to fix a bug, even if it's from 2010 :)).
@dfaure There seem to be a regression caused by this patch: if I reach the end of current view, and then press Enter so it has to scroll down, it scrolls to the very beginning of the document instead.
This is the same issue as in ancient bug 195828, and it seems like it is Qt issue.
Here's the simplest MWE:
#include <QApplication>
#include <QMainWindow>
#include <QTextEdit>
#include <QPushButton>
#include <QVBoxLayout>
class CustomTextEdit : public QTextEdit
{
public:
using QTextEdit::QTextEdit;
void keyPressEvent(QKeyEvent *e) override
{
textCursor().beginEditBlock();
QTextEdit::keyPressEvent(e);
textCursor().endEditBlock();
}
};
int main(int argc, char *argv[])
{
QApplication a(argc, argv);
auto w = new QWidget();
auto edit = new CustomTextEdit();
auto button = new QPushButton(QStringLiteral("Insert new line"));
button->connect(button, &QPushButton::clicked, [edit](){
auto cursor = edit->textCursor();
cursor.beginEditBlock();
cursor.insertBlock();
edit->setTextCursor(cursor);
cursor.endEditBlock();
});
auto layout = new QVBoxLayout(w);
layout->addWidget(edit);
layout->addWidget(button);
w->show();
return a.exec();
}Seems like the following code in QTextCursorPrivate::setX causes the trouble:
if (priv->isInEditBlock() || priv->inContentsChange) {
x = -1; // mark dirty
return;
}and seems like this "dirty" value is treated literally afterwards.
For this example, switching endEditBlock and setTextCursor lines fixes the issue (and same with Insert Rule).
I've also found a (dirty) workaround for the original patch: if I call setTextCursor(textCursor()) right after endEditBlock(), it works like a charm. It seems like this line (although semantically is a NOOP) triggers the update of this "dirty" value.
How should I proceed?
The *ideal* way of proceeding is to submit a Qt bug report with your testcase (after searching for an existing Qt bug report for the same issue), then turn it into a Qt autotest and make a gerrit merge request for Qt with both the test and a fix for it. And then, and only then, add a workaround in krichtextedit with a Qt version ifdef and a reference to the Qt bug report.
The less ideal way is to only do the Qt bug report and the workaround here...
Thanks for your comment!
I already did the first part (that is, Qt bugreport).
I can play around Qt code too, and see if I can prepare a test & patch.
Although, to be honest, I'm a bit afraid to touch it. Most likely there was some reasoning behind this "dirty" value, which I can't yet guess by looking at the code (it lacks documentation / comments) :(
BTW, there is a less dirty workaround: there is a nice method ensureCursorVisible(), which does the trick without any black magic. Probably that's what setTextCursor(textCursor()) does internally.