Bug Hunting Session
Bug 71428 - Other: Spelling of "turquise" in Impress Table Design (drawdoc4.cxx)
Summary: Other: Spelling of "turquise" in Impress Table Design (drawdoc4.cxx)
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: BSA target:4.2.0
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-09 13:32 UTC by Owen Genat (retired)
Modified: 2013-11-15 09:17 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot showing turquoise Table Design selection. (23.74 KB, image/png)
2013-11-09 13:32 UTC, Owen Genat (retired)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Genat (retired) 2013-11-09 13:32:10 UTC
Created attachment 88929 [details]
Screenshot showing turquoise Table Design selection.

Problem description: I noticed this by selecting the turquoise Table Design in Impress (refer screenshot) and then examining the XML which contained "turquise" rather than "turquoise".

Steps to reproduce:
1. Start Impress.
2. View > Toolbars > check Table.
3. Using the Table toolbar insert a table.
4. With the table selected, double-click the turquoise Table Design in the Tasks Pane.
5. Save ODP.
6. Examine content.xml to find it contains:

<table:table table:template-name="turquise"

Current behavior: The term "turquoise" is spelled "turquise".

Expected behavior: Spelling is "turquoise".

http://opengrok.libreoffice.org/xref/core/sd/source/core/drawdoc4.cxx indicates:

// ---- Turquoise --------------------------------------------------

   Any aTurquise1( implMakeSolidCellStyle( pSSPool, "turquise1" , aDefaultCellStyleName, RGB_COLORDATA(71,184,184)));
   Any aTurquise2( implMakeSolidCellStyle( pSSPool, "turquise2" , aDefaultCellStyleName, RGB_COLORDATA(51,163,163)));
   Any aTurquise3( implMakeSolidCellStyle( pSSPool, "turquise3" , aDefaultCellStyleName, RGB_COLORDATA(25,138,138)));

   implCreateTableTemplate( xTableFamily, "turquise" , aTurquise1, aTurquise3, aTurquise2 );

This seems to have been introduced in 7c6348f89d8f13623ce3f086304ddc3524cc47e1 on 2008-03-12 if I am reading the OpenGrok history correctly, so I set the version to Inherited from OOo. I hope I have that all correct.
Operating System: All
Version: Inherited From OOo
Comment 1 Owen Genat (retired) 2013-11-09 13:44:33 UTC
I accidentally set the component to Drawing when reporting the bug. Setting to Presentation.
Comment 2 Julien Nabet 2013-11-09 21:14:24 UTC
Owen: I submitted a patch here:
https://gerrit.libreoffice.org/#/c/6624/

Thank you for having noticed and declared this bug.
Comment 3 Owen Genat (retired) 2013-11-11 06:17:08 UTC
Excellent. The patch looks OK to my non-developer eyes. Good to be able to contribute something, however small. Thanks for the prompt fix Julien.
Comment 4 Commit Notification 2013-11-12 17:10:02 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

fdo#71428: Spelling of "turquise" in Impress Table Design



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 5 Thorsten Behrens (CIB) 2013-11-12 17:45:40 UTC
Eh. Sorry guys, but I politely disagree. The fix, as it is, causes a regression, when loading old documents in a fixed Impress, or new documents in an old Impress. In that case, the style icon in the table design pane gets duplicated (because Impress does not recognise the name anymore & therefore creates a brand-new style).

99.9% of the users do not look into the xml; most of then will be irritated by the dupe entry though.

If you still want this fixed, please catch the misspelled case at least on import (I'd personally probably just let it be. there's also the rather German 'seetang' style - I'd similarly advise translating that post-factum though).
Comment 6 Julien Nabet 2013-11-12 18:13:52 UTC
Thorsten: I agree, there'll be problems with old doc/new version and new doc/old version and that 99% of people never look xml but:
- With the time, old LO versions won't be used anymore so case 1 will disappear
- About old docs, certainly there may be some bugtrackers filled but, once we answered why and how to fix this, the information will be spread about it. What about putting some words about it on release notes and on some FAQs to help?

Also, just wondering, if it was for something more important (eg: ODF compliance or even more important), what should we do? 

Now, it's just my humble opinion so if you prefer to revert the patch, go ahead because:
- I don't know what to do to accept "turquise"
- I find it weird to put some code to deal with a fixed bug
Again, it's just my opinion, I don't pretend to know the right thing to do here :-)
Anyway, thank you for your useful review, I had thought there could be some problems (see my first comment in this gerrit) but didn't know precisely what they could be.
Comment 7 Thorsten Behrens (CIB) 2013-11-12 18:51:37 UTC
(In reply to comment #6)
> - With the time, old LO versions won't be used anymore so case 1 will
>   disappear
>
It's not only LO, it's the combined, installed base of OOo/LibO/AOO
since 2008, when the table feature got integrated. That's clearly >100
million copies, though hard to guess how many affected documents are
there. ;)

> - About old docs, certainly there may be some bugtrackers filled but, once
> we answered why and how to fix this, the information will be spread about
> it. What about putting some words about it on release notes and on some FAQs
> to help?
>
> Also, just wondering, if it was for something more important (eg: ODF
> compliance or even more important), what should we do? 
>
Yes, that is my point - between the two options we have (irritate
users, albeit temporarily, and having a spelling mistake in xml), in
this case I'd prefer the not-annoy-the-users one. For odf conformance
or interop issues, that verdict probably would be different. I
currently wonder how to explain & motivate that change to our users.
Comment 8 Julien Nabet 2013-11-12 19:04:54 UTC
Thorsten: I understand the fact it might irritate but it means that once a mistake is made and concerns the content of xml, no way to fix it (or just for rare cases) :-(
I'd prefer a fixed version even if it irritates but that's just me :-)

(Oups, forgot about OOo and AOO! (wasn't on purpose of course, really :-))

So I'll let you revert the patch and put the tracker at WONTFIX if you think it's better like this.
Comment 9 Owen Genat (retired) 2013-11-13 00:20:47 UTC
(In reply to comment #5)
> The fix, as it is, causes a regression, when loading old documents in a fixed
> Impress, or new documents in an old Impress. In that case, the style icon in 
> the table design pane gets duplicated (because Impress does not recognise the
> name anymore & therefore creates a brand-new style).

This was my greatest concern. Bit of a learning process for me. The behaviour indicated (duplicate Table Design style icons) is certainly undesirable and to be avoided. 

(In reply to comment #7)
> For odf conformance or interop issues, that verdict probably would be different. 
> I currently wonder how to explain & motivate that change to our users.

I was reporting this as a user as well as someone with an interest in ODF conformance / interoperability and so have divided loyalties on this issue. I would hope we can make LO as conforming as possible, but in a way that maintains compatibility for the existing user-base. I suppose I am a bit disappointed that small changes like this are so difficult to address in an easy manner, however I can also see that having a growing list of checks on import for all these small changes would become equally undesirable over time.

Thanks for checking this Thorsten. I am happy to defer to whatever the developers feel is best. At least it is now documented.
Comment 10 Thorsten Behrens (CIB) 2013-11-13 09:33:17 UTC
(In reply to comment #9)
> I was reporting this as a user as well as someone with an interest in ODF
> conformance / interoperability and so have divided loyalties on this
> issue.
>
This is neither a conformance, nor (beyond the UI glitch) an interop
question. It's just ugly xml. ;)
Comment 11 Thorsten Behrens (CIB) 2013-11-15 09:17:22 UTC
Nice release noting plus workaround from Julien is now at

https://wiki.documentfoundation.org/ReleaseNotes/4.2#ODF_changes

That settles it for me, thanks to all involved!