Bug 98211 - RTL: Paragraph dialog justified combobox doesnt list 'right'
Summary: RTL: Paragraph dialog justified combobox doesnt list 'right'
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Yousuf Philips (jay) (retired)
URL:
Whiteboard: target:6.1.0 target:6.0.0.1
Keywords: needsDevEval, topicUI
: 106345 (view as bug list)
Depends on:
Blocks: RTL-CTL Paragraph-Dialog Paragraph-Alignment
  Show dependency treegraph
 
Reported: 2016-02-26 20:31 UTC by Yousuf Philips (jay) (retired)
Modified: 2018-09-30 20:31 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 Yousuf Philips (jay) (retired) 2016-02-26 20:31:50 UTC
Steps:
1) Open Writer with CTL enabled in the options dialog
2) Click the 'Right-to-Left' button in the toolbar
3) Open Paragraph dialog and switch to Alignment tab
4) Select the 'Justified' radio button and notice that the 'Last Line' drop down menu only lists Left, Centered, and Justified.

If a paragraph is set to RTL then the 'Left' option should be replaced with 'Right' as RTL text auto aligns to the right and not the left.

Version: 5.2.0.0.alpha0+
Build ID: 003d0ccf902d2449320dd24119564565a384f365
CPU Threads: 2; OS Version: Linux 4.2; UI Render: default; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2016-02-23_02:42:33
Locale: en-US (en_US.UTF-8)
Comment 1 Usama 2016-03-14 15:13:37 UTC
Hello,

Thank you for reporting the bug. I can confirm that the bug is present in master.

However it seems that it's just a visual error. because the text is still aligned to right when you select left. It just follows the RTL LTR button direction.

Version: Version: 5.2.0.0.alpha0+ and on Version: 5.0.2.2
OS: Ubuntu 15.10
Comment 2 Yousuf Philips (jay) (retired) 2016-03-15 00:59:20 UTC
Hi Usama,

Yes its a error that 'Right' isnt included in the drop down and a visual error in the preview, as it shows left justified rather than right justified.

@Maxim: Simple fix?
Comment 3 Khaled Hosny (inactive) 2016-12-26 06:49:38 UTC
I’d just use direction-neutral word like “start” or “beginning” which would work for vertical text to, instead of left/right.
Comment 4 Yousuf Philips (jay) (retired) 2017-05-02 16:21:06 UTC
(In reply to Khaled Hosny from comment #3)
> I’d just use direction-neutral word like “start” or “beginning” which would
> work for vertical text to, instead of left/right.

Changing the label to a direction-neutral word would fix the labelling but the preview would still be incorrectly.
Comment 5 Yousuf Philips (jay) (retired) 2017-10-02 15:08:03 UTC
(In reply to Khaled Hosny from comment #3)
> I’d just use direction-neutral word like “start” or “beginning” which would
> work for vertical text to, instead of left/right.

Heiko, Stuart, Adolfo: what directionally neutral word do you think we should use?
Comment 6 V Stuart Foote 2017-10-02 16:12:37 UTC
(In reply to Yousuf Philips (jay) from comment #5)
> (In reply to Khaled Hosny from comment #3)
> > I’d just use direction-neutral word like “start” or “beginning” which would
> > work for vertical text to, instead of left/right.
> 
> Heiko, Stuart, Adolfo: what directionally neutral word do you think we
> should use?

In en-SU think "origin" would be another choice, but agree we should shift from left/right that has be correctly toggled now depending on locale and script direction and does nothing for the vertical layouts controlled with the same widget.

"Begining" might be too long, while "start" connotes there is a stop.  Also we should ask the translation community ... @Sophie?
Comment 7 Heiko Tietze 2017-10-02 17:24:37 UTC
Inner/outer is used on pages. Onset, outset as alternatives to init.
Comment 8 Yousuf Philips (jay) (retired) 2017-10-06 14:35:18 UTC
In a recent build, someone has changed left to 'left/top' and right to 'right/bottom' on the tabs for alignment and tabs, assuming to deal with cjk languages that write top to bottom.
Comment 9 V Stuart Foote 2017-10-06 16:38:57 UTC
(In reply to Yousuf Philips (jay) from comment #8)
> In a recent build, someone has changed left to 'left/top' and right to
> 'right/bottom' on the tabs for alignment and tabs, assuming to deal with cjk
> languages that write top to bottom.

No that labeling was already there when Oliver did the conversion to .ui

https://cgit.freedesktop.org/libreoffice/core/commit/?id=324141f21bf2280d7613c4056ee8cd997ea345f9

But guess something more than direction neutral labeling could (should?) be implemented. Something to detect any of the four common paragraph text layout modes LRTB, RLTB, TBRL, TBLR in use and adapt the UI for the paragraph (alignment, tabs) and the page (text direction).
Comment 10 Yousuf Philips (jay) (retired) 2017-10-06 16:52:11 UTC
(In reply to V Stuart Foote from comment #9)
> No that labeling was already there when Oliver did the conversion to .ui

Yes thats my bad. It is showing up for me as i have asian language support enabled, but ideally we should fix those labels as well, because labels like 'Left/Top' arent good in my view.
Comment 11 V Stuart Foote 2017-10-06 17:25:04 UTC
(In reply to Yousuf Philips (jay) from comment #10)
> Yes thats my bad. It is showing up for me as i have asian language support
> enabled, but ideally we should fix those labels as well, because labels like
> 'Left/Top' arent good in my view.

True needs attention, but the Left/Top, Right/Bottom labels for Tabs are hard coded in .ui, and not linked to CJK use.  

Also, some interesting affects in the ruler(s) not following the vertical text direction(s) when set in the Page dialog.
Comment 12 Yousuf Philips (jay) (retired) 2017-10-07 08:58:20 UTC
(In reply to V Stuart Foote from comment #11)
> True needs attention, but the Left/Top, Right/Bottom labels for Tabs are
> hard coded in .ui, and not linked to CJK use.  

As CJK users would enable asian language, it is somewhat linked to CJK use.

(In reply to V Stuart Foote from comment #9)
> But guess something more than direction neutral labeling could (should?) be
> implemented. Something to detect any of the four common paragraph text
> layout modes LRTB, RLTB, TBRL, TBLR in use and adapt the UI for the
> paragraph (alignment, tabs) and the page (text direction).

Yes a solution is needed to be done in the paragraph dialog to handle all 4 possible layouts. Please file a bug for this.

So when asian language support is enabled, the 'Left' entry in the drop down menu is called 'Default', so i guess we can just go with that.

https://gerrit.libreoffice.org/43224
Comment 13 Heiko Tietze 2017-10-15 06:54:44 UTC
Don't like the 'Default' solution in https://gerrit.libreoffice.org/43224 as the user would need to understand what that means (left, right, top but not justified or bottom for instance).

We should better make the text depending on the regional setting. Sounds to me like an easyhack that could be fixed by Jim. 

So my take is, if the variable text can be implemented easily we should do that, otherwise take the Default solution.
Comment 14 Khaled Hosny (inactive) 2017-10-15 08:28:51 UTC
(In reply to Heiko Tietze from comment #13)
> We should better make the text depending on the regional setting. Sounds to
> me like an easyhack that could be fixed by Jim. 

What is “regional setting”?
Comment 15 Khaled Hosny (inactive) 2017-10-15 08:31:19 UTC
(In reply to V Stuart Foote from comment #6)
> (In reply to Yousuf Philips (jay) from comment #5)
> > (In reply to Khaled Hosny from comment #3)
> > > I’d just use direction-neutral word like “start” or “beginning” which would
> > > work for vertical text to, instead of left/right.
> > 
> > Heiko, Stuart, Adolfo: what directionally neutral word do you think we
> > should use?
> 
> In en-SU think "origin" would be another choice, but agree we should shift
> from left/right that has be correctly toggled now depending on locale and
> script direction and does nothing for the vertical layouts controlled with
> the same widget.
> 
> "Begining" might be too long, while "start" connotes there is a stop.  Also
> we should ask the translation community ... @Sophie?

CSS uses start/end for neutral direction-sensitive alignment:
https://developer.mozilla.org/en-US/docs/Web/CSS/text-align
Comment 16 Khaled Hosny (inactive) 2017-10-15 08:32:37 UTC
(In reply to Khaled Hosny from comment #15)
> (In reply to V Stuart Foote from comment #6)
> > (In reply to Yousuf Philips (jay) from comment #5)
> > > (In reply to Khaled Hosny from comment #3)
> > > > I’d just use direction-neutral word like “start” or “beginning” which would
> > > > work for vertical text to, instead of left/right.
> > > 
> > > Heiko, Stuart, Adolfo: what directionally neutral word do you think we
> > > should use?
> > 
> > In en-SU think "origin" would be another choice, but agree we should shift
> > from left/right that has be correctly toggled now depending on locale and
> > script direction and does nothing for the vertical layouts controlled with
> > the same widget.
> > 
> > "Begining" might be too long, while "start" connotes there is a stop.  Also
> > we should ask the translation community ... @Sophie?
> 
> CSS uses start/end for neutral direction-sensitive alignment:
> https://developer.mozilla.org/en-US/docs/Web/CSS/text-align

The same for last line:
https://developer.mozilla.org/en-US/docs/Web/CSS/text-align-last
Comment 17 Heiko Tietze 2017-10-15 13:52:05 UTC
(In reply to Khaled Hosny from comment #14)
> What is “regional setting”?

locale or Tools > Options > Language Settings > Languages > Locale Settings (or default language for documents)
Comment 18 Yousuf Philips (jay) (retired) 2017-10-15 17:29:03 UTC
(In reply to Heiko Tietze from comment #13)
> Don't like the 'Default' solution in https://gerrit.libreoffice.org/43224 as
> the user would need to understand what that means (left, right, top but not
> justified or bottom for instance).

We also use 'Default' in the alignment tab of the format cell dialog in Calc, so if user understands it there, it is fine in Writer.

https://i.stack.imgur.com/jCY1E.png

(In reply to Khaled Hosny from comment #15)
> CSS uses start/end for neutral direction-sensitive alignment:
> https://developer.mozilla.org/en-US/docs/Web/CSS/text-align

Yep they use start and end, but we wont be using end and as stuart mentioned, start maybe translated incorrectly.

(In reply to Heiko Tietze from comment #17)
> locale or Tools > Options > Language Settings > Languages > Locale Settings
> (or default language for documents)

It isnt locale specific, as you can change the direction of a single paragraph to be RTL in a LTR document.
Comment 19 Heiko Tietze 2017-10-15 18:35:18 UTC
(In reply to Yousuf Philips (jay) from comment #18)
> It isnt locale specific, as you can change the direction of a single
> paragraph to be RTL in a LTR document.

Have you considered my actual comment with your reply?
Comment 20 Khaled Hosny (inactive) 2017-10-16 03:11:48 UTC
(In reply to Heiko Tietze from comment #17)
> (In reply to Khaled Hosny from comment #14)
> > What is “regional setting”?
> 
> locale or Tools > Options > Language Settings > Languages > Locale Settings
> (or default language for documents)

But the alignment is not specific to locale but to paragraph direction. You can be using an Arabic locale and writing LTR text, and vice versa.
Comment 21 Khaled Hosny (inactive) 2017-10-16 03:14:43 UTC
(In reply to Yousuf Philips (jay) from comment #18)
> (In reply to Khaled Hosny from comment #15)
> > CSS uses start/end for neutral direction-sensitive alignment:
> > https://developer.mozilla.org/en-US/docs/Web/CSS/text-align
> 
> Yep they use start and end, but we wont be using end and as stuart
> mentioned, start maybe translated incorrectly.

I don’t find this convincing. Default also does not say much nor handle all cases.
Comment 22 Adolfo Jayme 2017-10-16 04:36:16 UTC
Let’s keep “Left” for LTR languages and add a hidden label for RTL languages simply stating “Right”, and swap it dynamically. This has been done in other dialogs for special cases, like the PDF dialog, which changes its usual “Pages” label to “Slides” when opened from Impress.
Comment 23 Heiko Tietze 2017-10-16 08:55:36 UTC
My take was

* just "Default" is confusing
* for new documents RTL users want "Right" by default, LTR'lers "Left"
* alignment depends on the document source (wouldn't use the paragraph language) and we should override the default 'Right' or 'Left' with this actual value
Comment 24 Yousuf Philips (jay) (retired) 2017-10-16 13:10:01 UTC
(In reply to Heiko Tietze from comment #19)
> Have you considered my actual comment with your reply?

Yes i have but maybe we are misunderstanding each other.

(In reply to Khaled Hosny from comment #21)
> I don’t find this convincing. Default also does not say much nor handle all
> cases.

Did you find it confusing also in Calc? 'Default' to me means 'default to the inherited alignment', which should handle all cases.

(In reply to Adolfo Jayme from comment #22)
> Let’s keep “Left” for LTR languages and add a hidden label for RTL languages
> simply stating “Right”, and swap it dynamically. This has been done in other
> dialogs for special cases, like the PDF dialog, which changes its usual
> “Pages” label to “Slides” when opened from Impress.

So 'Left' for LTR, 'Right' for RTL and 'Default' for CJK and add additional coding to show the appropriate label. That seems like alot of work for very little benefit, which can easily be resolved by having a unified label and unify the label in writer and calc.

(In reply to Heiko Tietze from comment #23)
> * just "Default" is confusing

So i'm assuming you want to change it in Calc as well.
Comment 25 Heiko Tietze 2017-10-16 14:47:58 UTC
(In reply to Yousuf Philips (jay) from comment #24)
> So i'm assuming you want to change it in Calc as well.

That would be great. And of course everything depend on the effort. Otherwise 'Default' is better than 'Left' while 'Right' is correct.
Comment 26 Jim Raykowski 2017-10-16 16:54:10 UTC
Heiko thank you for suggesting me to implement this. Jay, do you got this one? I see it is assigned to you. Let me know if I can help.
Comment 27 Yousuf Philips (jay) (retired) 2017-10-16 20:17:29 UTC
(In reply to Jim Raykowski from comment #26)
> Heiko thank you for suggesting me to implement this. Jay, do you got this
> one? I see it is assigned to you. Let me know if I can help.

I assigned myself to it based on my patch in comment 12, but you are welcome to it if you want to add the 'Right' label to the drop down, and i'll abandon my patch.
Comment 28 Jim Raykowski 2017-10-16 23:31:46 UTC
What should be the used in the 'Last line' drop down when 'Use superordinate object settings' is selected in the 'Text direction' drop down? 'Left' or 'Right' or 'Default' or maybe both 'Left' and 'Right' or something else dependent on 'superordinate object settings'?
Comment 29 V Stuart Foote 2017-10-17 04:27:58 UTC
No, here we should retain the simpler static implementation--little return for the added dialog complexity this would require adjust GUI to changes in locale and script assigned to the paragraph. 

Believe "Start", from the Start <--> End position pair would be the correct static replacement for "Left", rather than "Default".

As Khaled notes, Start <--> End are used in CSS.  Likewise given our extensive use of ICU libraries for determining paragraph direction by locale and scirpt, seems we should also follow Unicode practice.  There Start and End are used extensively in Unicode [1] for segmentation describing boundaries [2].

Perhaps we should provide a note to translators, otherwise not worry about the Start <--> Stop action coordinate pair this could be confused for.

=-ref-=
[1] http://unicode.org/reports/tr29/#Introduction
[2] grapheme clusters (GB1|GB2), words (WB1|WB2), sentences (SB1|SB2), or even paragraphs (with use of U+2029, U+2028 should we want to adopt it).
Comment 30 Yousuf Philips (jay) (retired) 2017-10-17 06:14:39 UTC
(In reply to V Stuart Foote from comment #29)
> Believe "Start", from the Start <--> End position pair would be the correct
> static replacement for "Left", rather than "Default".

Fine lets go with that. Patch has been adjusted to use it for LTR, RTL and CJK, so its unified.

> Perhaps we should provide a note to translators, otherwise not worry about
> the Start <--> Stop action coordinate pair this could be confused for.

@Sophie: How would we do this?

(In reply to Jim Raykowski from comment #28)
> What should be the used in the 'Last line' drop down when 'Use superordinate
> object settings' is selected in the 'Text direction' drop down? 'Left' or
> 'Right' or 'Default' or maybe both 'Left' and 'Right' or something else
> dependent on 'superordinate object settings'?

Sorry Jim, but i'll keep this one. I know Heiko will be able to find you another easy hack to work on.
Comment 31 Heiko Tietze 2017-10-17 06:58:46 UTC
(In reply to V Stuart Foote from comment #29)
> No, here we should retain the simpler static implementation...

Why? Have you read the other comments? 

It's not that Start/End (better than Default) is bad. But if possible we should at least try to implement usable solutions.
Comment 32 Jim Raykowski 2017-10-17 07:01:01 UTC
OK but for the most part I have the dynamic last line done including correct preview.

Here is how it behaves -

Text direction: -> Last line:
Use superordinate... -> Start
Left-to-right (LTR) -> Left
Right-to-left (RTL) -> Right

Had fun hacking in the paragrph code and watching the discussion.
Comment 33 Khaled Hosny (inactive) 2017-10-17 12:08:44 UTC
(In reply to Jim Raykowski from comment #32)
> OK but for the most part I have the dynamic last line done including correct
> preview.
> 
> Here is how it behaves -
> 
> Text direction: -> Last line:
> Use superordinate... -> Start
> Left-to-right (LTR) -> Left
> Right-to-left (RTL) -> Right
> 
> Had fun hacking in the paragrph code and watching the discussion.

The behavior here should match what Writer is actually doing (whatever it is), so this should reference the relevant Writer code (at least in source code comment). If this can use existing code that would be even better, otherwise I don’t feel easy about the dialog trying to emulate Writer with the potential of future mismatch when Writer implementation is altered in anyway.
Comment 34 V Stuart Foote 2017-10-17 14:33:51 UTC
(In reply to Heiko Tietze from comment #31)
> (In reply to V Stuart Foote from comment #29)
> > No, here we should retain the simpler static implementation...
> 
> Why? Have you read the other comments? 
> 
> It's not that Start/End (better than Default) is bad. But if possible we
> should at least try to implement usable solutions.

I don't disagree, but issue here is only to have a label appropriate for the default handling of last line alignment--"Left" must go. 

(In reply to Heiko Tietze from comment #23)
> ... 
>* alignment depends on the document source (wouldn't use the paragraph
> language) and we should override the default 'Right' or 'Left' with this
> actual value

No it has to be at the paragraph level with its locale and script. And behavior now in our implementation is the "Default" for the locale and script of the paragraph. The source is a bit convoluted and implemented multiple times (chart, frames, tables--a search for WritingMode2).

Since it is a positional value Start/End, taking the start by default, rather than trying to identify Top/Bottom/Left/Right, keep it simple and just use Start.

Refactoring, perhaps to fully match the CSS handling, and make use of ICU library Unicode segmentation [1] would be a worthwhile enhancement--much bigger issue than this. 

=-ref-=
[1] http://userguide.icu-project.org/boundaryanalysis
Comment 35 Jim Raykowski 2017-10-17 19:51:52 UTC
If interested I will commit what I have done. If no use for the first entry dependency in the last line list box, maybe the last line list box and fixed text enabling and preview fixes can be cherry picked.

The patch is wrote so whatever word is decided on for default use can be simply changed in the paragalignpage.ui. Currently it is 'Start'.
Comment 36 Yousuf Philips (jay) (retired) 2017-10-18 20:48:52 UTC
@Jim: yes push your patch to gerrit and set it to -1, so it doesn't get pushed until there is some agreement here. I've -1 my patch as well.

Would be great if you could fix the preview bugs in any of these bug, as no debates are going on there :D - bug 98212, bug 107567, bug 107643.
Comment 37 Jim Raykowski 2017-10-19 07:10:41 UTC
(In reply to Yousuf Philips (jay) from comment #36)
> @Jim: yes push your patch to gerrit and set it to -1, so it doesn't get
> pushed until there is some agreement here. I've -1 my patch as well.
> 
> Would be great if you could fix the preview bugs in any of these bug, as no
> debates are going on there :D - bug 98212, bug 107567, bug 107643.

patch pushed to gerrit and set to -1. bug 98212 fix is included in patch. bug 107567 and 107643 seem to refer to same preview bug in paragraph indent and spacings tab. Great to be able to help!
Comment 38 Yousuf Philips (jay) (retired) 2017-10-19 11:16:24 UTC
(In reply to Jim Raykowski from comment #37)
> patch pushed to gerrit and set to -1. bug 98212 fix is included in patch.
> bug 107567 and 107643 seem to refer to same preview bug in paragraph indent
> and spacings tab. Great to be able to help!

Would be great if you could separate the fix for bug 98212 as a separate patch so we can push that in, whatever the end result of this bug.
Comment 39 Yousuf Philips (jay) (retired) 2017-10-19 11:17:22 UTC
Jim's patch: https://gerrit.libreoffice.org/#/c/43522/
Comment 40 Omer Zak 2017-11-11 20:20:22 UTC
Still happens in:

Version: 6.0.0.0.alpha1+
Build ID: 5d12237d79f289a1dcf8e07aa03df329e136f078
CPU threads: 8; OS: Linux 4.9; UI render: default; VCL: gtk3; 
Locale: en-US (en_US.utf8); Calc: group

OS: Debian 64bit Stretch (Debian 9.2, with some backported packages)
Comment 41 Heiko Tietze 2017-11-29 20:17:23 UTC
Topic has been discussed in the design meeting and the suggestion of "Start" instead of "Left" was accepted (also for CJK).
Comment 42 Commit Notification 2017-12-02 10:07:22 UTC
Yousuf Philips committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98211 make last line default value language neutral

It will be available in 6.1.0.

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 43 Yousuf Philips (jay) (retired) 2017-12-02 10:15:23 UTC
Jim: when you get time, can you remove the CJK enabled code in the drop down justified combobox, so we can have just a single 'Start' entry in the gtkliststore rather than two. thanks.
Comment 44 Commit Notification 2017-12-03 19:33:32 UTC
Yousuf Philips committed a patch related to this issue.
It has been pushed to "libreoffice-6-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8aafeb3c72b79acffab0c608e8f4037a80296f53&h=libreoffice-6-0

tdf#98211 make last line default value language neutral

It will be available in 6.0.0.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.
Comment 45 Yousuf Philips (jay) (retired) 2017-12-18 12:38:45 UTC
*** Bug 106345 has been marked as a duplicate of this bug. ***
Comment 46 Xisco Faulí 2018-01-18 10:01:24 UTC
A polite ping to Yousuf Philips (jay): is this bug fixed? if so, could you
please close it as RESOLVED FIXED ? Thanks
Comment 47 Lior Kaplan 2018-09-30 20:31:35 UTC
Version 6.1.1.2