Bug 131812 - Exporting to html doesn't preserve RTL property
Summary: Exporting to html doesn't preserve RTL property
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.4.2.2 release
Hardware: All All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:7.0.0 target:6.4.4
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-02 07:19 UTC by Zayed
Modified: 2020-05-10 13:37 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
ODF file (23.34 KB, application/vnd.oasis.opendocument.text)
2020-04-02 07:19 UTC, Zayed
Details
Exported HTML (5.87 KB, text/html)
2020-04-02 07:20 UTC, Zayed
Details
Screenshot of original ODF and HTML file (479.01 KB, image/png)
2020-04-02 07:21 UTC, Zayed
Details
Test export (5.85 KB, text/html)
2020-04-09 20:11 UTC, Julien Nabet
Details
another test (5.92 KB, text/html)
2020-04-11 08:33 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zayed 2020-04-02 07:19:37 UTC
Created attachment 159251 [details]
ODF file

when  exporting ODF file with RTL property set to HTML, the HTML file set to be LTR in the <body>.

The correct behavior is to maintain the RTL property.
Comment 1 Zayed 2020-04-02 07:20:26 UTC
Created attachment 159252 [details]
Exported HTML
Comment 2 Zayed 2020-04-02 07:21:19 UTC
Created attachment 159253 [details]
Screenshot of original ODF and HTML file
Comment 3 Julien Nabet 2020-04-02 18:02:43 UTC
If you check Page Style, Text direction is LTR, not RTL.
The text is only right justified.

So yes html should be right justified, but nothing related to Text direction.
Comment 4 Julien Nabet 2020-04-02 18:08:22 UTC
Don't know what to think, when I unzipped the file, I noticed that content.xml was containing this:
  <office:automatic-styles>
    <style:style style:name="P1" style:family="paragraph" style:parent-style-name="Standard">
      <style:paragraph-properties fo:text-align="end" style:justify-single-word="false" style:writing-mode="rl-tb" />
    </style:style>
    <style:style style:name="P2" style:family="paragraph" style:parent-style-name="Heading_20_1">
      <style:paragraph-properties fo:text-align="end" style:justify-single-word="false" style:writing-mode="rl-tb" />
    </style:style>
    <style:style style:name="P3" style:family="paragraph" style:parent-style-name="Heading_20_2">
      <style:paragraph-properties fo:text-align="end" style:justify-single-word="false" style:writing-mode="rl-tb" />
    </style:style>
  </office:automatic-styles>
Comment 5 Dieter 2020-04-08 20:32:55 UTC
(In reply to Julien Nabet from comment #3)
> If you check Page Style, Text direction is LTR, not RTL.
> The text is only right justified.
> 
> So yes html should be right justified, but nothing related to Text direction.

Zayed, can you confirm that?

=> NEEDINFO
Comment 6 Zayed 2020-04-09 06:53:51 UTC
(In reply to Julien Nabet from comment #3)
> If you check Page Style, Text direction is LTR, not RTL.
> The text is only right justified.
> 
> So yes html should be right justified, but nothing related to Text direction.

All the paragraphs are RTL direction in Writer and right justified. 
Currently the Body is like this:
<body dir="ltr" style="max-width:8.5in;margin-top:0.7874in; margin-bottom:0.7874in; margin-left:0.7874in; margin-right:0.7874in; ">

and CSS for P is:
.P1 { font-size:12pt; font-family:Liberation Serif; writing-mode:rl-tb; text-align:left ! important;}

The correct one should be with out dir="ltr" for the body, like this:
<body  style="max-width:8.5in;margin-top:0.7874in; margin-bottom:0.7874in; margin-left:0.7874in; margin-right:0.7874in; ">

and for the P like this:
.P1 { font-size:12pt; font-family:Liberation Serif; writing-mode:rl-tb; text-align:right ! important;}

I hope this clear by now.
Comment 7 Julien Nabet 2020-04-09 20:11:54 UTC
Created attachment 159453 [details]
Test export

It seems that .P1 change is sufficient.

In this case, this patch could make it:
diff --git a/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl b/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
index da123b1e5146..2564c95657ed 100644
--- a/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
+++ b/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
@@ -123,10 +123,10 @@
             <xsl:when test="contains(., 'end')">
                 <xsl:choose>
                     <xsl:when test="parent::*/@style:writing-mode and contains(parent::*/@style:writing-mode, 'rl')">
-                        <xsl:text>text-align:left ! important;</xsl:text>
+                        <xsl:text>text-align:right ! important;</xsl:text>
                     </xsl:when>
                     <xsl:otherwise>
-                        <xsl:text>text-align:right ! important; </xsl:text>
+                        <xsl:text>text-align:left ! important; </xsl:text>
                     </xsl:otherwise>
                 </xsl:choose>
             </xsl:when>
Comment 8 Zayed 2020-04-09 20:50:47 UTC
This will solve one part. the other part the direction. As per this
https://developer.mozilla.org/en-US/docs/Web/CSS/writing-mode

writing-mode: rl-tb is deprecated it should be horizontal-tb. 

After that we need to set the direction CSS property to be direction: rtl;

and the last is how we can set the direction CSS property for <body>. I suggest to be same as language of the LibreOffice GUI.
Comment 9 QA Administrators 2020-04-10 03:32:57 UTC Comment hidden (obsolete)
Comment 10 Julien Nabet 2020-04-11 08:33:20 UTC
Created attachment 159485 [details]
another test

Here's another test.

The patch would be:
diff --git a/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl b/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
index da123b1e5146..3a83ec200a06 100644
--- a/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
+++ b/filter/source/xslt/odf2xhtml/export/common/styles/style_mapping_css.xsl
@@ -123,10 +123,10 @@
             <xsl:when test="contains(., 'end')">
                 <xsl:choose>
                     <xsl:when test="parent::*/@style:writing-mode and contains(parent::*/@style:writing-mode, 'rl')">
-                        <xsl:text>text-align:left ! important;</xsl:text>
+                        <xsl:text>text-align:right ! important;</xsl:text>
                     </xsl:when>
                     <xsl:otherwise>
-                        <xsl:text>text-align:right ! important; </xsl:text>
+                        <xsl:text>text-align:left ! important; </xsl:text>
                     </xsl:otherwise>
                 </xsl:choose>
             </xsl:when>
@@ -290,8 +290,20 @@
     </xsl:template>
     <xsl:template match="@style:writing-mode">
         <xsl:text>writing-mode:</xsl:text>
-        <xsl:value-of select="."/>
-        <xsl:text>; </xsl:text>
+        <xsl:choose>
+            <xsl:when test=".='rl-tb'">
+                <xsl:text>horizontal-tb; direction=rtl; </xsl:text>
+            </xsl:when>
+            <xsl:when test=".='lr-tb'">
+                <xsl:text>horizontal-tb; direction=ltr; </xsl:text>
+            </xsl:when>
+            <xsl:when test=".='tb-rl'">
+                <xsl:text>vertical-rl; </xsl:text>
+            </xsl:when>
+            <xsl:when test=".='tb-lr'">
+                <xsl:text>vertical-lr; </xsl:text>
+            </xsl:when>
+        </xsl:choose>
     </xsl:template>
     <!-- *** Properties with a no 'fo:' or 'style:' prefix *** -->
     <xsl:template match="@table:align">
Comment 11 Zayed 2020-04-11 19:08:57 UTC
Almost there, you need to put direction:rtl; instead of direction=rtl;
Comment 12 Julien Nabet 2020-04-11 20:38:51 UTC
(In reply to Zayed from comment #11)
> Almost there, you need to put direction:rtl; instead of direction=rtl;

Indeed, I submitted this patch to review:
https://gerrit.libreoffice.org/c/core/+/92067

For body part, I let it aside because I'm not sure how to do this.
However if, after this patch, you have an example triggered by it, don't hesitate to submit a new bugtracker about it.
Comment 13 Commit Notification 2020-04-11 21:25:14 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/95460224f4f8443547dbf5cedbea008ea2639337

tdf#131812: fix exporting to html doesn't preserve RTL property

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Zayed 2020-04-12 17:29:29 UTC
(In reply to Commit Notification from comment #13)
> Julien Nabet committed a patch related to this issue.
> It has been pushed to "master":
> 
> https://git.libreoffice.org/core/commit/
> 95460224f4f8443547dbf5cedbea008ea2639337
> 
> tdf#131812: fix exporting to html doesn't preserve RTL property
> 
> It will be available in 7.0.0.
> 
> The patch should be included in the daily builds available at
> https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
> information about daily builds can be found at:
> https://wiki.documentfoundation.org/Testing_Daily_Builds
> 
> Affected users are encouraged to test the fix and report feedback.

Great. Thank you.
Comment 15 Xisco Faulí 2020-04-15 17:47:39 UTC
Verified in

Version: 7.0.0.0.alpha0+
Build ID: 24e1563b5db3de5abac0d4fd0f737ec319e703e8
CPU threads: 4; OS: Linux 4.19; UI render: default; VCL: gtk3; 
Locale: en-US (en_US.UTF-8); UI-Language: en-US
Calc: threaded

Besides, I've created a unittest for it < https://gerrit.libreoffice.org/c/core/+/92315 >
@Julien, Thanks for fixing this issue. Should it be closed this issue as RESOLVED FIXED ?
Comment 16 Julien Nabet 2020-04-15 17:53:52 UTC
(In reply to Xisco Faulí from comment #15)
> ...
> @Julien, Thanks for fixing this issue. Should it be closed this issue as
> RESOLVED FIXED ?

Yes.
I just cherry-picked the patch for 6.4 branch, here:
https://gerrit.libreoffice.org/c/core/+/92019
Comment 17 Commit Notification 2020-04-16 08:19:55 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/f57962fe9a282a3acd0c824e283316b38fe7a130

tdf#131812: xhtml: Add unittest

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2020-04-16 08:20:06 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/f7c6660dc2dd60ba8df7ee63a63438dea1368e7c

tdf#131812: fix exporting to html doesn't preserve RTL property

It will be available in 6.4.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 BogdanB 2020-05-10 13:26:15 UTC
Great! Works well on
Version: 7.0.0.0.alpha1
Build ID: 6a03b2a54143a9bc0c6d4c7f1...
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3; 
Locale: ro-RO (ro_RO.UTF-8); UI: en-US
Calc: threaded
Comment 20 BogdanB 2020-05-10 13:37:36 UTC
Tested also on
Version: 6.4.4.1
Build ID: b50bc319eca5cd5b66fbfe2ebd0d3bd1eed099b5
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3; 
Locale: ro-RO (ro_RO.UTF-8); UI-Language: en-US
Calc: threaded