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: 4.0.6.2 release
Hardware: All All
: medium major
Assignee: Not Assigned
QA Contact:
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:
i915 platform:
i915 features:


Attachments

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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.