Bug 54629 - EDITING: Hebrew/Arabic text is not clickable in textbox area of Impress slides (Lohit Hindi font only)
Summary: EDITING: Hebrew/Arabic text is not clickable in textbox area of Impress slide...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Localization (show other bugs)
Version:
(earliest affected)
3.6.0.4 release
Hardware: Other All
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:4.1.0 target:4.0.1
Keywords:
Depends on:
Blocks: RTL-Textbox
  Show dependency treegraph
 
Reported: 2012-09-07 09:34 UTC by Arie Skliarouk
Modified: 2017-10-12 15:47 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Test document for fdo#54629 (14.81 KB, application/vnd.oasis.opendocument.presentation)
2012-09-29 09:46 UTC, Lior Kaplan
Details
test document that shows the bug (17.30 KB, application/vnd.oasis.opendocument.presentation)
2012-09-29 16:50 UTC, Arie Skliarouk
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arie Skliarouk 2012-09-07 09:34:15 UTC
* Create a slide in impress
* Create (or type in an existing) textbox both english and hebrew text
* Make the textbox to be out of focus
* Observe the correct behavior that the textbox can be focused if clicking on english text. The cursor will be positioned correctly at the point closest to clicking point.
* Make the textbox to be out of focus
* Observe incorrect behavior the the text box *can not* be focused if clicking on hebrew text.
Comment 1 Faisal Menawer 2012-09-08 09:11:36 UTC
Hi,

I can reproduce this on Arabic language also using Libreoffice 3.6 on fedora 16-64bit.

Thanks
  Faisal
Comment 2 Roman Eisele 2012-09-29 08:53:34 UTC
@ Arie Skliarouk:

Thank you for your bug report!
But please specify:
-- Which LibreOffice version do you use?
-- Which Platform/Operating System do you use? 


@ Faisal Menawer:

Thank you for confirming this behaviour! I have added “Arabic” to the Summary according to your results.


Hint:

I can NOT reproduce this with LibreOffice 3.6.2.2 (Build ID: da8c1e6) on Mac OS X, but this probably just means that this is a platform-specific bug.
Comment 3 Lior Kaplan 2012-09-29 09:46:41 UTC
Created attachment 67844 [details]
Test document for fdo#54629

Attached is a test document created by the instructions above. 

I couldn't reproduce the problem with LibreOffice 3.5.4.2 
Build ID: 350m1(Build:2) from Debian unstable packages on an 64bit machine or LibreOffice 3.6.0.4 (Build ID: 932b512) from official builds on an Ubuntu 10.04 64bit machine.
Comment 4 Arie Skliarouk 2012-09-29 16:49:25 UTC
The bug shows up only when the hebrew text is typed using font "Lohit Hindi" (not sure about arabic).

I have no idea how I ended up using the font though.

Find attached the test document where the bug can be seen.
Comment 5 Arie Skliarouk 2012-09-29 16:50:22 UTC
Created attachment 67854 [details]
test document that shows the bug
Comment 6 Lior Kaplan 2012-09-30 11:11:08 UTC
Ok, when changing the font to Lohit Hindi I can reproduce the bug. This font comes from the "ttf-indic-fonts-core" package in Ubuntu.
Comment 7 Caolán McNamara 2013-01-23 14:49:03 UTC
In MultiSalLayout::GetBoundRect we loop through each fallback font and union each level's GetBoundRect to get the result, which seems reasonable, except that for each level that means a call to SalLayout::GetBoundRect which calls X11SalGraphics::GetGlyphBoundRect.

Those sublevel SalLayouts don't know that they are sublevels so they never set the level bits on their glyphs. Which results in X11SalGraphics::GetGlyphBoundRect always searching for that glyph in the level 0 font, so there's a mismatch between the font the SalLayout's are created to use and the font that gets used.

Now, SalLayout::GetBoundRect is a fairly simple thing, it just calls GetNextGlyphs 1 by 1 and X11SalGraphics::GetGlyphBoundRect on the result of that.  So I *think* that if we remove the specialized MultiSalLayout::GetBoundRect in favour of the default implementation, then because MultiSalLayout::GetNextGlyphs is specialized to do-the-right-thing and set up the glyph ids correctly, then we bubble the ids down to X11SalGraphics::GetGlyphBoundRect correctly to get it to use the right font and we get the correct bounding size.
Comment 8 Not Assigned 2013-01-23 14:52:21 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "master":

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

fdo#54629 MultiSalLayout::GetBoundRect always uses level 0 fallback font



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 Caolán McNamara 2013-01-23 14:55:17 UTC
please test this, the MultiSalLayout is a real pain to get right.
Comment 10 Lior Kaplan 2013-01-23 21:39:08 UTC
Verified on master, thanks for the fix (:

Any reason not to cherry pick to the 4-0 branch (for 4.0.1)?
Comment 11 Ahmad Harthi 2013-01-23 22:47:35 UTC
I've an issue writing LTR in an RTL context while editing, Kaplan can you please check that and see if it cause any problems for you?
Comment 12 Lior Kaplan 2013-01-23 23:02:06 UTC
@Ahmad - could you be more specific ? Maybe give some instructions what to try?
Comment 13 Ahmad Harthi 2013-01-26 09:43:32 UTC
(In reply to comment #12)
> @Ahmad - could you be more specific ? Maybe give some instructions what to
> try?

Kaplan, the patch works great!!
Just I didn't know I had stumbled by another bug after I applied the patch by Caolán! (Sorry)

I've added you and Caolán to the CC list for 59892:
https://bugs.freedesktop.org/show_bug.cgi?id=59892
Comment 14 Caolán McNamara 2013-01-28 10:17:44 UTC
yeah, this is worth a shot for 4-0, though not for 4-0-0
Comment 15 Not Assigned 2013-02-03 09:05:29 UTC
Caolan McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-4-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9b70bf62e6b5319e282cd3533c90216aabccfe53&h=libreoffice-4-0

fdo#54629 MultiSalLayout::GetBoundRect always uses level 0 fallback font


It will be available in LibreOffice 4.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.