Bug 53023 - AutoText (text only) embeds extra carriage return
Summary: AutoText (text only) embeds extra carriage return
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Andreas Heinisch
URL:
Whiteboard: target:7.5.0
Keywords: difficultyMedium, needsDevEval, needUITest
: 63740 77482 147670 (view as bug list)
Depends on:
Blocks: AutoText
  Show dependency treegraph
 
Reported: 2012-08-01 01:40 UTC by Jay Valatka
Modified: 2022-12-07 21:08 UTC (History)
12 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jay Valatka 2012-08-01 01:40:06 UTC
Creating user shortcuts with AutoText seems to append a carriage return to the end of the text saved.  Removing the extra carriage return from the item through direct editing and saving it will not remove the carriage return; it just reappears upon save.

1. First create a new category or just use "My AutoText" with Edit | AutoText... | Categories...
2. Inside a document, type in a few words, like "ready to close year" _without adding a carriage return_.  Highlight the words and do a Ctrl-C (copy to clipboard).
3. Go back to Edit | AutoText... and enter a Name and Shortcut.  Click on AutoText drop-down button and select "New (text only)".

If you select "New" instead of "New (text only)", you may not see this problem.

4. Go back to document to try the shortcut you've just created.
Comment 1 Jean-Baptiste Faure 2012-09-02 08:32:31 UTC
Old bug, already there in OOo 2 : https://issues.apache.org/ooo/show_bug.cgi?id=79861
Set version accordingly : 3.3.0

Best regards. JBF
Comment 2 Joel Madero 2014-05-16 20:09:46 UTC
*** Bug 77482 has been marked as a duplicate of this bug. ***
Comment 3 a07cd040897db54e103c 2014-05-19 06:27:01 UTC
*** Bug 63740 has been marked as a duplicate of this bug. ***
Comment 4 Joel Madero 2014-05-20 05:12:08 UTC
In the future please do not add bugs to most annoying bug list without knowing what belongs on there - I am removing it from the list.

Note that other bugs on MAB list are mostly crashers, hangers, or if not, are bugs that are going to affect a large portion of our users. This bug has 4 total comments over almost 2 year period - it's really not affecting many users.

Please do not add it back to the list - it won't make it get fixed faster, it'll just make the MAB list less useful because bugs that don't belong on there will be sitting on the list.
Comment 5 a07cd040897db54e103c 2014-05-20 07:23:06 UTC
(In reply to comment #4)
> This bug has
> 4 total comments over almost 2 year period - it's really not affecting many
> users.

Did you also have a look at the "See also" section?

At least two duplicates of this bug exist (bug 63740 and bug 77482) which in total add 17 more comments. *Yourself* commented there and even marked 77482 as a duplicate of this. So dont't complain about too less users affected and comments in this certain report.

The apache bugtracker for AOO adds 11 further comments to this issue.

So we are talking about 33 comments in total about a bug that at least exists since 2007-07-21 in AOO and was taken over to LO.

> Note that other bugs on MAB list are mostly crashers, hangers, or if not,
> are bugs that are going to affect a large portion of our users

The fact, that at least two further users felt it would be worth writing a report and 33 comments about the reports make me feel believe that "large portion of our users" *are* affected. 

Furthermore i believe that a bug

  - that makes a function unusable in some cases (single phrases without formatting) 
  - that is known of for nearly *seven* years now
  - that is reported many times
  - that is easily reproducible
  - that seems easy to fix

definitely *is* "most annoying".

> Please do not add it back to the list

I won't, but your arguments are wrong.
Comment 6 QA Administrators 2015-06-08 14:41:28 UTC Comment hidden (obsolete)
Comment 7 Laurent Balland 2015-06-08 16:52:09 UTC
Confirmed on Win 7 with Version: 4.4.4.1.0+
Build ID: 24c5f9979e61fde7b098af60756a4890e5713390
Locale : fr_FR
Comment 8 Laurent Balland 2015-12-06 21:30:24 UTC
Possible entry point:
http://opengrok.libreoffice.org/xref/core/sw/source/core/doc/docglos.cxx#InsertGlossary
Comment 9 royerjy 2016-08-08 16:02:25 UTC
Always in LO 2.0.4. Is it so complicated to correct this bug ?
Comment 10 a07cd040897db54e103c 2016-08-10 15:30:02 UTC
just retestet with 

Version: 5.1.4.2 (x64)
Build-ID: f99d75f39f1c57ebdd7ffc5f42867c12031db97a
CPU-Threads: 2; BS-Version: Windows 6.1; UI-Render: Standard; 
Gebietsschema: de-DE (de_DE)

and the problem still exists
Comment 11 Eduardo Sanchez 2016-10-08 14:31:29 UTC
Confirmed in 5.2.1.2 under Gnu/Linux.

Please fix. The bug is extremely annoying and it's making me lose valuable time, focus, and keystrokes. Thanks!
Comment 12 Laurent Balland 2017-01-22 11:20:45 UTC
An easy workaround is to use autocorrection instead of autotext
Comment 13 QA Administrators 2018-08-18 02:39:07 UTC Comment hidden (obsolete)
Comment 14 Jean-Baptiste Faure 2018-08-18 07:20:15 UTC
Still reproducible with LO 6.1.1.0.0+ built at home under Ubuntu 18.04 x86-64.

Best regards. JBF
Comment 15 Justus 2019-07-05 21:39:17 UTC
Still annoying and reproducible on Arch-Linux with LO:

- Version: 6.2.4.2.0+
- Build-ID: 6.2.4-1

Best regards
Justus
Comment 16 myrtleslopes 2020-08-21 12:39:04 UTC
This is an important bug for me too, and I imagine it is for lots of other translators. It is one of the main reasons I have not changed to Libreoffice from Winword.
Comment 17 Eyal Rozenberg 2021-04-01 14:49:21 UTC
(In reply to myrtleslopes from comment #16)

Adding my voice to the request for this to get some attention. No changes as of LO 7.1.2.1. 

Also, is it really "difficultyMedium" rather than "easyHack"? Doesn't sound like it's too difficult.
Comment 18 Regina Henschel 2022-02-26 20:08:07 UTC
*** Bug 147670 has been marked as a duplicate of this bug. ***
Comment 19 Mike Kaganski 2022-12-02 14:12:10 UTC
(In reply to Eyal Rozenberg from comment #17)
> Also, is it really "difficultyMedium" rather than "easyHack"?

"difficultyFoo" are only used with easyhacks, as their difficulty level. When appear without the easyhack keyword, it's either put by mistake, or was accidentally kept when easyhack keyword was removed. (The redundancy is another topic, my explanation is about actual use, not how it would possibly be engineered best.)

> Doesn't sound like it's too difficult.

FTR: given how it's implemented, fixing this (which https://gerrit.libreoffice.org/c/core/+/143353 does) will be a breaking change for anyone who created their own text-only autotexts, *expecting* the trailing paragraph mark. So despite it's not too complex technically, respect regression reports.
Comment 20 Eyal Rozenberg 2022-12-02 14:59:19 UTC
(In reply to Mike Kaganski from comment #19)
> FTR: given how it's implemented, fixing this (which
> https://gerrit.libreoffice.org/c/core/+/143353 does) will be a breaking
> change for anyone who created their own text-only autotexts, *expecting* the
> trailing paragraph mark. So despite it's not too complex technically,
> respect regression reports.

s/respect/expect/ ?

Well... it's true that it will technically be breaking, but I'm guessing a person who knows how to create autotext entries is likely to figure out what's going on, and recreate their entries. Still, breakage is breakage. Is there a policy on what to do when a feature's behavior/semantics change?
Comment 21 Andreas Heinisch 2022-12-02 17:02:00 UTC
It surely will break some of the already generated auto texts. However, it is easily possible to recreate the auto text in a matter of seconds, but it is - without this patch - impossible to remove the empty parageraph at the end of an auto text.
Comment 22 Mike Kaganski 2022-12-02 17:16:33 UTC
(In reply to Eyal Rozenberg from comment #20)
> s/respect/expect/ ?

Yes, exactly; sorry.

> Still, breakage is breakage. Is
> there a policy on what to do when a feature's behavior/semantics change?

No formal policy AFAIK; we just can't backport this to 7.4. Changing behavior with major releases is OK. And indeed, changing this is very reasonable; the only point of comment 19 is to be prepared.

Still, we at least must check our bundled autotexts, and make sure that at least *those* work as before (so e.g. the lorem one needs an explicit trailing newline now).
Comment 23 Andreas Heinisch 2022-12-02 17:55:42 UTC
I don't know if the currently installed auto texts should be the same or not. For instance, the "A" + F3 key now produces "Attention" without a new paragraph and previously it produced "Attention" with a new paragraph. What does a user expect inserting such an only text auto text?
Comment 24 Eyal Rozenberg 2022-12-02 18:08:11 UTC
(In reply to Andreas Heinisch from comment #23)
> What does a user expect inserting such an only text auto text?

Autotext with a key that doesn't include a paragraph break  is never expected to add a paragraph break is always unexpected - for a user who is not already used to what we have implemented right now. The idea is that an autotext behaves like acronym expansion, while paragraph breaks are something structural. (The exception is when the autotext key itself suggests a paragraph break explicitly, e.g. "para" or "break" or "nextpar" etc. - and we don't have those.)
Comment 25 Andreas Heinisch 2022-12-02 18:55:11 UTC
(In reply to Mike Kaganski from comment #22)
> Still, we at least must check our bundled autotexts, and make sure that at
> least *those* work as before (so e.g. the lorem one needs an explicit
> trailing newline now).

There are a lot of auto texts within different languages and for instance in the en-US at least the following have to be changed:

Header Commemorative Publication
Header Left Commemorative Publication
Lorem
Quote
VIA OVERNIGHT MAIL

Strange that the paragraphs in between are lost.
Comment 26 Mike Kaganski 2022-12-02 20:13:27 UTC
(In reply to Andreas Heinisch from comment #23)
> For instance, the "A" + F3 key now produces "Attention" without a new
> paragraph and previously it produced "Attention" with a new paragraph. What
> does a user expect inserting such an only text auto text?

Any existing user, who uses it already, expects what has always happened before. Any new user would learn whatever the program does for any autotext that they haven't used yet. So IMO: keep the paragraph after "Attention:". IMO, it's like the "Best wishes" (BW), which is naturally followed by a newline, and then the user types the name.

(In reply to Eyal Rozenberg from comment #24)
> Autotext with a key that doesn't include a paragraph break  is never
> expected to add a paragraph break is always unexpected - for a user who is
> not already used to what we have implemented right now.

I don't see any logic behind this: the autotext is inserted with a keyword + F3. How would a user know if "autotext's key includes a paragraph break"? Or is the logic then that no autotext ever may include a paragraph break - because no "key" could include it?

An autotext inserting something like a heading (or even large text body with many paragraphs) is perfectly normal and widespread. The idea that autotext is just a shortcut for a keyword expanding to a few words is too narrow. Anyhow, if a specific autotext now has a trailing newline, but is better without, it's a matter for a separate enhancement, which this change will make possible.
Comment 27 Mike Kaganski 2022-12-03 08:32:07 UTC
Let me clarify myself: I don't claim that all our autotexts must keep the trailing newline forever: I only say that the change that fixes *this specific issue* must make as few side-effect functional changes as possible; this means: it must no more insert the newlines for autotexts that were created from selections without such trailing newline (the very essense of the issue); it necessarily would break custom autotexts (unavoidable breakage); but at least it must make sure to keep bundled autotexts' behavior unchanged - *for all of them*, regardless of the original intent of those bundled autotexts' authors (so avoiding what can be avoided). Then, later commits could be created to selectively change specific bundled autotexts, with respective bug reports, UX discussion, and a way for anyone to put their reasonings in those respective issues, both before and after those changes, and a way to selectively revert those changes if needed.
Comment 28 Eyal Rozenberg 2022-12-03 20:49:42 UTC
(In reply to Mike Kaganski from comment #26)

I'll also start with a clarification - I don't mind very much whether the pre-defined autotext entries keep the extra paragraph break or not. Like Andreas said - recreating them is very easy, so whether they're defined this way or the other by default, I don't mind very much (nor will I think people will be particularly annoyed either way).


> I don't see any logic behind this: the autotext is inserted with a keyword +
> F3. How would a user know if "autotext's key includes a paragraph break"? Or
> is the logic then that no autotext ever may include a paragraph break -
> because no "key" could include it?

It's mostly the latter. My perception (and I believe most users' perception) of  autotext is something intended for inline, within-the-paragraph, replacements. I think of applying autotext entry like expanding an acronym or shorthand for an expression: "ltd." -> "limited", "asap" -> as soon as possible, and perhaps the production of non-directly-typeable characters with multi-char keys, e.g. "---" -> "—" (em dash). Expansion involving paragraph breaks is surprising to me.

But again - I don't mind how the predefined entries are, I'll just define what I want how I want it.
Comment 29 Andreas Heinisch 2022-12-04 11:17:01 UTC
I will change all the auto texts so that they keep the same functionality as before. Afterwards, they are easy to change if a lot of users are convinced they should be changed.
Comment 30 Mike Kaganski 2022-12-06 09:23:43 UTC
Fixed with ce64398653a896c8a48dd6cabe2b75d9c025873d (commit notifications got a day off today).

Please create a release notes entry; make a notice there that users who relied on automatic newline insertion will need to re-create their custom text-only autotexts, selecting the trailing empty paragraph.

Thanks Andreas!
Comment 31 Andreas Heinisch 2022-12-06 10:14:39 UTC
Added to https://wiki.documentfoundation.org/ReleaseNotes/7.5#Writer
Comment 32 Eyal Rozenberg 2022-12-06 19:11:56 UTC
Andreas, I'm looking at that commit. Your message says:

> The carriage return at the end of it is appended in
> SwXMLTextBlockParContext::~SwXMLTextBlockParContext()
> which can't be removed without introducing side effects.

and so you remove it after-the-fact in sw/source/core/doc/docglos.cxx

but - is that the right way to do it? That is, should the addition of the extra paragraph be avoided in the first place? I looked at SwXMLTextBlockParContext:

https://github.com/LibreOffice/core/blob/10d29c390dd58ed629dd27fe5ed35fae28eceec3/sw/source/core/swg/SwXMLBlockImport.cxx#L105

and that class doesn't have any doxygen explaining what it's about. I did see the `\015` added in the destructor - also without explanation. Perhaps it's not the right class to use for text-only autotext entries? Or, alternatively - are you sure that other code which uses this class needs the extra newline  I mean, I appreciate the speedy fix, but still.
Comment 33 Andreas Heinisch 2022-12-07 07:24:56 UTC
Thx Eyal for your remarks! In fact I tried to not append the extra paragraph in the first place in the first patch set https://gerrit.libreoffice.org/c/core/+/143353/1, but it turned out that some of the auto texts did not work as expected, and opening some text files didn't as well.

So I decided the easiest way to fix this is to remove the supplementary new line, because text blocks are also used for auto corrections and insertions of text blocks.

Glossary entries and auto corrections start a new document using BeginGetDoc [1] and create a new document using the new line at the end including the used text. 

Another possibility to solve this issue could be a new parameter passed down to MakeTextBlock or a new flag in [2] in order to not include the new line at the end.

[1] https://opengrok.libreoffice.org/xref/core/sw/source/core/doc/docglos.cxx?r=ce643986#141
[2] https://opengrok.libreoffice.org/xref/core/sw/source/core/inc/swblocks.hxx?r=61f42c31#42

Mike what do you think about the flag in [2]?
Comment 34 Andreas Heinisch 2022-12-07 11:04:00 UTC
Apparently the UI-Tests hang, so I revert this patch for now, and try to pass down a flag in order to prevent the newline at the end of an autotext.
Comment 35 Commit Notification 2022-12-07 12:22:20 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/36a554e1f1c4e924a2cbc2df5a4f164fa31e3d6b

Revert "tdf#53023 - Remove last empty paragraph from auto text"

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 36 Commit Notification 2022-12-07 14:43:37 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3772f18266c3347e8fd60eb8f058d65328c400b4

tdf#53023 - Remove last empty paragraph from auto text

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 37 Andreas Heinisch 2022-12-07 18:20:32 UTC
Hi Eyal! I tried your proposal to check if we can avoid the additional new line at the end of the text in the dtor, but this causes to many side effects, because the new line needs to be avoided just for the very last text.

So atm, I have no better fix for this problem where even myself sometimes run into it :)
Comment 38 Commit Notification 2022-12-07 21:06:20 UTC
Andreas Heinisch committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4d83725981e46c62efb5ce6a68ae7c345465b448

tdf#53023 - Revert hanging unit test

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 39 Andreas Heinisch 2022-12-07 21:08:53 UTC
Rational behind the decision of partially reverting the ui test can be found at https://gerrit.libreoffice.org/c/core/+/143781