Issue 91465 - SwTxtNode::GetLang() ignores nScript parameter
Summary: SwTxtNode::GetLang() ignores nScript parameter
Status: CLOSED FIXED
Alias: None
Product: Writer
Classification: Application
Component: code (show other issues)
Version: DEV300m19
Hardware: PC All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: frank.meies
QA Contact: issues@sw
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-08 12:17 UTC by hennerdrewes
Modified: 2013-08-07 14:43 UTC (History)
2 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
patch for file sw/source/core/txtnode/thints.cxx (1.18 KB, patch)
2008-07-08 12:19 UTC, hennerdrewes
no flags Details | Diff
sample doc (20.18 KB, application/vnd.oasis.opendocument.text)
2008-07-10 14:40 UTC, hennerdrewes
no flags Details
modified patch (1.11 KB, patch)
2008-07-10 16:28 UTC, hennerdrewes
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description hennerdrewes 2008-07-08 12:17:39 UTC
While debugging my changes of issue 60591, I noticed an apparent bug in
SwTxtNode::GetLang(). If pSwpHints == NULL, the function will not evaluate the
nScript parameter, and eventually return a LATIN language, when CTL was requested. 

In my debugging case, it disrupted the kashida justification when mixing English
and Arabic in a text line. I think this could also disrupt justification of
other CTL or Asian languages.

Attaching a patch.
Comment 1 hennerdrewes 2008-07-08 12:19:45 UTC
Created attachment 55006 [details]
patch for file sw/source/core/txtnode/thints.cxx
Comment 2 michael.ruess 2008-07-08 16:03:23 UTC
MRU->AMA: please have a look at this patch proposal. If you are not the correct
developer for this, please reassign to proper person, thanks!
Comment 3 andreas.martens 2008-07-09 10:37:18 UTC
ama->fme: Please have a look!
Comment 4 frank.meies 2008-07-10 06:30:16 UTC
fme->hennerdrewes: Thank you for your patch. I'll have a look.
Comment 5 hennerdrewes 2008-07-10 06:53:57 UTC
@fme: When I was debugging the kashida justification, I had the following case: 
In one line I had English text followed by Arabic, separated by a blank. The
formatting of the line was completely broken, words were placed beyond the page
margin.

This didn't happen with my kashida patches in 2.4.1 code, only in dev300_xxx.
When  the justification type is determined by asking for the language attribute,
the nScript parameter gets ignored if pSwpHints == NULL. (Apparently there was a
pSwpHints in 2.4.x, but now there is not.) As nScript was ignored, and break
iterator looked only at the first character of the Arabic run (the blank), the
Latin language is returned.

As this will probably also happen for other special justifications (e.g. Thai),
it might be a good idea to include this fix in 3.0.
Comment 6 frank.meies 2008-07-10 12:58:25 UTC
fme->hennerdrewes: Thank you for your patch. Yes, I think you are right, the
code needs to be changed here. But what's puzzling me is this:

[...] In one line I had English text followed by Arabic, separated by a blank.
[...] As nScript was ignored, and break iterator looked only at the first
character of the Arabic run (the blank), the Latin language is returned. [...]

I wonder why the blank that's located between the English and Arabic text is
part of the Arabic portion instead of the English portion. I think the first
character of a portion usually should be of the same script type as the rest for
the portion. Therefore this shouldn't be much of a problem. Can you attach a
document?
Comment 7 hennerdrewes 2008-07-10 14:25:16 UTC
I worked with the kashida.odt document from the other Arabic issues, I'll attach
it again. 

But there might be another problem involved: I deliberately changed CTL language
attributes on selected text ranges (to Hebrew) in the document to see, how the
justification reacts on that. 

Expected results: kashida justification should switch to regular blank
justification. 

Observed results: Apparently my language choices were not always applied to the
text. Sometimes I still had blank justification, although the status bar
reported Arabic (Egypt) as the current language. At a certain stage, the
spellchecker marked Arabic words: My conclusion: somewhere in the program the
LATIN language was reported instead of the CTL (Arabic) language. (I don't have
a spellchecker for neither Arabic or Hebrew installed, so it must be LATIN).

Finally, I managed to get rid of this chaos by selecting all the text and
applying Arabic (Jordan) to it, Arabic (Egypt) didn't work. 

I don't know if you will manange to reproduce my observings with the original
document. Also my kashida patches might influence the results, though I doubt
that the language chaos is influenced by this.
Comment 8 hennerdrewes 2008-07-10 14:40:57 UTC
Created attachment 55051 [details]
sample doc
Comment 9 frank.meies 2008-07-10 16:15:05 UTC
fme->hennerdrewes: Well, I can't see a problem with the bugdoc in a current
DEV300m23.
Comment 10 hennerdrewes 2008-07-10 16:28:31 UTC
Created attachment 55053 [details]
modified patch
Comment 11 hennerdrewes 2008-07-10 16:33:42 UTC
You are right. 

Apparently the problems I observed are connected to the kashida changes I made.
So this issue is probably less critical than I thought. Anyhow, with the
modifications to the patch I just posted, everything seems to work fine with my
kashida patches
Comment 12 frank.meies 2008-09-29 13:30:24 UTC
@hennerdrewes: Thank you for your patch!
Comment 13 frank.meies 2008-11-27 10:43:51 UTC
Verified by developer.
Comment 14 thorsten.ziehm 2010-02-22 14:46:58 UTC
This issue is closed automatically. It should be fixed in a version with is
available for longer than half a year (OOo 3.1). If you think this issue isn't
fixed in the current version (OOo 3.2) please reopen it. But then please pay
attention about the field 'target milestone'.
The closure was approved by the Release Status Meeting at 22nd of February 2010
and it is based on the issue handling guideline for fixed/verified issues  :
http://wiki.services.openoffice.org/wiki/Handle_fixed_verified_issues