Differential D28964

[KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f)
ClosedPublic
Actions

Authored by poboiko on Apr 18 2020, 9:22 PM.

Details

Reviewers
dfaure
ahmadsamir
Group Reviewers
Frameworks
Commits
R310:0494fc4a2838: [KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f)
Summary

This patch removes the workaround, which (according to comment) comes from Qt4
times. Although I wasn't even able to find the original bug (only the title,
"Remove implicit margin on bullet lists when exporting to HTML"), it would seem
like the workaround is no longer actual (i.e. margins are consistent).

This is an alternative approach to D28819: [KRichTextEdit] Always treat key press as single modification in undo stack, as it removes the code which
actually cluttered the undo stack. The latter patch introduced couple
regressions unfortunately
(e.g. ability to "undo" a written word, not just the last letter)

However, it slightly changes the behavior: it removes margins around a bullet
list, which are probably not needed anyways (e.g. LibreOffice doesn't have it)

Test Plan
  1. Create a bullet list in krichtexteditor with couple of lines
  2. Try to undo -> a single undo is sufficient
  3. Press enter to add bunch of empty new lines, so that scroll appears
  4. Scroll still follows the cursor

Diff Detail

Repository
R310 KTextWidgets
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
poboiko created this revision.Apr 18 2020, 9:22 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 18 2020, 9:22 PM
poboiko requested review of this revision.Apr 18 2020, 9:22 PM
dfaure added a comment.Apr 18 2020, 9:33 PM

The qt4 task was probably Nokia-internal before the issue tracker was open. But here are more details if you didn't have them:

commit 3eb65f48c97dc5e1a8980afa35bea8e4b16f08ba
Author: Eskil Abrahamsen Blomfeldt <eblomfel@trolltech.com>
Date: Fri Sep 4 15:05:46 2009 +0200

Remove implicit margin on bulleted lists when exporting to HTML

When we export a bulleted list to HTML, we add a meta-block around the
list items to set the bullet style of the list (<ul> or <ol>.) This
block will implicitly have a margin, even if the list itself does not
in the internal document. The result is that exporting a list to HTML
and then reloading it will give a different apperance, because the 0
margin of the original was not saved. The fix is to always set the
meta-block's margin to 0, since it is only there to set the bullet
style and should not affect the layout at all.

Task-number: 201228
Reviewed-by: Simon Hausmann
dfaure accepted this revision.Apr 18 2020, 9:35 PM

In any case it makes sense to remove a workaround for a known-to-be-fixed bug. Good find.

This revision is now accepted and ready to land.Apr 18 2020, 9:35 PM
Closed by commit R310:0494fc4a2838: [KRichTextWidget] Remove ancient workaround and fix regression (commit 1d1eb6f) (authored by poboiko). · Explain WhyApr 18 2020, 9:56 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 80506

View Options

src/widgets/krichtextedit.cpp

Loading...
View Options

src/widgets/nestedlisthelper.cpp

Loading...
View Options

src/widgets/nestedlisthelper_p.h

Loading...