Bug 81641 - Cancel style creation still creates a new style
Summary: Cancel style creation still creates a new style
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.0.6.2 release
Hardware: All All
: medium major
Assignee: Not Assigned
URL:
Whiteboard: NoRepro:4.0.0.0.beta1:Ubuntu NoRepro:...
Keywords: regression
Depends on:
Blocks:
 
Reported: 2014-07-22 10:24 UTC by Kevin Suo
Modified: 2014-07-29 12:41 UTC (History)
3 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 Kevin Suo 2014-07-22 10:24:58 UTC
Steps to reproduce:
1. F11;
2. Right-click -> New Style
3. Cancle

--> A new style named "untitled1" is still created.

Expected: When cancle, no new style is created.

Reproducible with:
Windows XP SP3
Version: 4.4.0.0.alpha0+ 2014-07-21_06:04:54, and Version 4.2.6.1
Comment 1 tommy27 2014-07-22 12:41:43 UTC
confirmed under WinXP 32bit using 4.2.5.2
we should retest against early 4.2.x and late 4.1.x release to see if it's a regression
Comment 2 Julien Nabet 2014-07-22 21:08:30 UTC
On pc Debian x86-64 with master sources updated 2 days ago, I could reproduce this.
In comparison Calc styles dialog doesn't have this problem.

sw:
- sw/uiconfig/swriter/ui/templatedialog2.ui
- sw/source/ui/fmtui/tmpdlg.cxx
- sfx2/source/dialog/styledlg.cxx

calc: 
- sc/uiconfig/scalc/ui/paratemplatedialog.ui
- sc/source/ui/styleui/styledlg.cxx
- sfx2/source/dialog/styledlg.cxx ?



I noticed some differences between both parts
- sw has an "Ok" method
- GetRefreshedSet() in sw is not virtual
- sw : pInSet->SetParent( &GetStyleSheet().GetItemSet() );
  whereas sc : pItemSet->SetParent( GetStyleSheet().GetItemSet().GetParent() );

But found nothing about cancel. I must recognize I don't understand why cancel doesn't work in sw.

Caolan: any idea?
Comment 3 Caolán McNamara 2014-07-23 13:29:11 UTC
I believe the style is created when the dialog appears, and on cancel "undo(1)" is called which is supposed to undo adding the style
Comment 4 Kevin Suo 2014-07-23 14:56:04 UTC
Do not reproduce in 3.6.7.2 --> REGRESSION.
Comment 5 Julien Nabet 2014-07-23 16:01:30 UTC
(In reply to comment #3)
> I believe the style is created when the dialog appears, and on cancel
> "undo(1)" is called which is supposed to undo adding the style

Indeed, I noticed this on console:
warn:legacy.osl:4274:1:svl/source/undo/undo.cxx:711: SfxUndoManager::Undo: undo stack is empty!
Comment 6 Kevin Suo 2014-07-23 16:10:33 UTC
Reproduce with 4.0.6.2
Not reproduce with 4.0.0 beta1.
Comment 7 Caolán McNamara 2014-07-24 08:54:07 UTC
Looks like a comedy of errors here

a) the style is created as a "conditional" style, conditional style creation has no undo implemented, which is an error and we should implement that.
b) it shouldn't be a conditional style in the first place
c) undo in master shows a format change to the newly created style, which shouldn't be shown in undo
d) fixing a or b and creating the style, then deleting the style and then undoing the three steps (including the annoying change style) crashes
Comment 8 Commit Notification 2014-07-24 11:49:23 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1af0e46102350114dd5e854b7692c640dae2727f

Resolves: fdo#81641 the new style shouldn't be a conditional style



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2014-07-24 11:49:38 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2223ff6cb99df097a357674801835c7a260b551d

Related: fdo#81641 exclude 'all styles' category from organizer page



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2014-07-24 11:49:53 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=183bde5bf5aa048792880292ac77777577fcd13b

Related: fdo#81641 implement undo of Conditional Text style creation



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2014-07-24 11:50:07 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e904562af54545684b32d2042ded6bdb9459edca

Related: fdo#81641 create new styles with an initial name



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Caolán McNamara 2014-07-24 12:23:44 UTC
ok, so the style should be created immediately, appear in the list, but on cancel get removed. It should not be a conditional style, and if we instead ok and delete it manually, then undoing and redoing should not crash.

I didn't tackle the ugly extra "change style" undo. Its fairly cosmetic and there's enough to be getting on with. 

https://gerrit.libreoffice.org/#/c/10502/ for 4-3
https://gerrit.libreoffice.org/#/c/10503/ for 4-2
Comment 13 Commit Notification 2014-07-29 12:41:00 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=30ef30970073b969430011da25e9412ffc217e8b&h=libreoffice-4-2

Related: fdo#81641 create new styles with an initial name


It will be available in LibreOffice 4.2.7.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2014-07-29 12:41:35 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1cf2cf1d4fbb1622c1789cb893d5e9fd9de1cd61&h=libreoffice-4-3

Resolves: fdo#81641 the new style shouldn't be a conditional style


It will be available in LibreOffice 4.3.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.