Bug 63351 - [test case] text-indent in Hebrew HTML pages moves right instead of left
Summary: [test case] text-indent in Hebrew HTML pages moves right instead of left
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Unclassified
Component: khtml renderer (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal with 40 votes (vote)
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-08-28 04:44 UTC by giantsloth
Modified: 2004-10-24 05:42 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
attaching testcase (179 bytes, text/html)
2004-10-24 05:42 UTC, Germain Garand
Details

Note You need to log in before you can comment on or make changes to this bug.
Description giantsloth 2003-08-28 04:44:19 UTC
Version:            (using KDE KDE 3.1.3)
Installed from:    Debian testing/unstable Packages
Compiler:          gcc 3.3 
OS:          Linux

In block items with dir="rtl" and text-indent with a positive value, the first line is moved to the right.

Expected behaviour: Logic, Mozlla and MS Explorer dictate that it should move to the left.

testcase:
<html>
<body align="right" dir="rtl" style="text-indent:3em;">
This line should be indented to the left<br>
This line is not indented
</body>
</html>

I suspect (might be wrong) that in kdelibs/khtml/rendering/render_flow.cpp
in RenderFlow::rightOffset()
the line 
"right += style()->textIndent().minWidth(cw);"
should have -= insetad of +=

I
Comment 1 giantsloth 2004-07-08 14:42:52 UTC
The suggested fix compiles in KDE 3.2.3, and it fixes the problem.

I don't have a suitable machine for compiling CVS on, but anyway, here is an untested diff against CVS HEAD. It's probably not the right format as I don't have a local CVS copy. Any chance to see this fixed in KDE 3.3?


--- kdelibs/khtml/rendering/render_block.cpp    2004-07-08 15:22:13.000000000 +0300
+++ render_block.cpp    2004-07-08 15:23:56.000000000 +0300
@@ -1505,9 +1505,9 @@
     if (applyTextIndent &&  m_firstLine && style()->direction() == RTL ) {
         int cw=0;
         if (style()->textIndent().isPercent())
             cw = containingBlock()->contentWidth();
-        right += style()->textIndent().minWidth(cw);
+        right -= style()->textIndent().minWidth(cw);
     }

     //kdDebug( 6040 ) << "rightOffset(" << y << ") = " << right << endl;
     return right;

Comment 2 Germain Garand 2004-10-24 05:42:00 UTC
Created attachment 8011 [details]
attaching testcase

patch looks right, sorry for the late review.
Comment 3 Germain Garand 2004-10-24 05:42:32 UTC
CVS commit by ggarand: 

Applying patch from giantsloth at fastmail.fm
Inverted logic for text-indent in RTL

BUG:63351


  M +1 -1      render_block.cpp   1.51


--- kdelibs/khtml/rendering/render_block.cpp  #1.50:1.51
@@ -1602,5 +1602,5 @@ RenderBlock::rightRelOffset(int y, int f
         if (style()->textIndent().isPercent())
             cw = containingBlock()->contentWidth();
-        right += style()->textIndent().minWidth(cw);
+        right -= style()->textIndent().minWidth(cw);
     }