331846 - Can't indent/outdent RTL blocks in Composer and Design Mode

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Uri Bernstein]

In Composer, trying to use the indent/outdent buttons on an RTL paragraph or block produces no visible result.
The same is true in design mode (Midas), where the "indent" and "outdent" commands appear to have no effect on RTL blocks.

What happens is that "indent" and "outdent" are implemented by adjusting the margin-left property. This should be changed to margin-right when the block direction is RTL.

This does not affect Thunderbird/MailNews, where blockquotes are used for indentation.

Comment 1

[Uri Bernstein]

Attachment: testcase

This is a simple RTL paragraph. Open it in Composer, or copy the source into the "View HTML Source" mode of Midas (http://www.mozilla.org/editor/midasdemo/), and try using the "indent" and "outdent" buttons.

Comment 2

[Uri Bernstein]

I should add that this only happens in useCSS mode (e.g. when "Use CSS" is checked in the Midas demo). This is also probably why it does not happen in Thunderbird/Mail.

Comment 3

[Uri Bernstein]

Attachment: patch

I never touched this code before, so I'm probably doing something the wrong way here, but anyway, I think this is the general idea for fixing this.

Comment 4

[Uri Bernstein]

Comment on attachment 216404 [details] [diff] [review]
patch

Timeless, are you good for reviewing this? If not, who is?

Comment 5

[Uri Bernstein]

Attachment: patch v2

Fixes requested by timeless on IRC:
- Changed the function name to MarginPropertyAtomForIndent
- Avoid "else" after return
- Fixed some nearby indentation

Comment 6

[Uri Bernstein]

Checked in:

Checking in editor/libeditor/base/nsEditPropertyAtomList.h;
/cvsroot/mozilla/editor/libeditor/base/nsEditPropertyAtomList.h,v  <--  nsEditPropertyAtomList.h
new revision: 1.7; previous revision: 1.6
done
Checking in editor/libeditor/html/nsHTMLEditRules.cpp;
/cvsroot/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp,v  <--  nsHTMLEditRules.cpp
new revision: 1.324; previous revision: 1.323
done

Comment 7

[Brian Rakowski]

Any chance of getting this in before FF3?  This bug keeps the "indent more" and "indent less" buttons of Gmail from working with Hebrew text.

Comment 8

[Mike Schroepfer]

BZ or timeless thoughts on safety of this patch for 1.8.1 or 1.8.0 branches?

Comment 9

[Uri Bernstein]

(In reply to comment #8)
> BZ or timeless thoughts on safety of this patch for 1.8.1 or 1.8.0 branches?
> 

My opinion, as the patch author, is that it is safe enough to go in to 1.8.1, and probably even to 1.8.0. But BZ's and timeless' opinions count here more, of course.

Comment 10

[Simon Montagu :smontagu]

Comment on attachment 216566 [details] [diff] [review]
patch v2

This has baked on the trunk for almost 2 months now. Especially now that the Hebrew interface for gmail has been launched, it would be good to have in the next release.

Comment 11

[Simon Montagu :smontagu]

Sorry, that should read "Hebrew and Arabic interfaces"

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 216566 [details] [diff] [review]
patch v2

Looks reasonable.

Comment 13

[Uri Bernstein]

Checked in to 1.8 branch:

/cvsroot/mozilla/editor/libeditor/base/nsEditPropertyAtomList.h,v  <--  nsEditPropertyAtomList.hnew revision: 1.4.12.2; previous revision: 1.4.12.1
done
Checking in mozilla/editor/libeditor/html/nsHTMLEditRules.cpp;
/cvsroot/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp,v  <--  nsHTMLEditRules.cpp
new revision: 1.320.4.2; previous revision: 1.320.4.1
done

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 216566 [details] [diff] [review]
patch v2

approved for 1.8.0 branch, a=dveditz for drivers

Comment 15

[Uri Bernstein]

Checked in to 1.8.0 branch:

Checking in mozilla/editor/libeditor/base/nsEditPropertyAtomList.h;/cvsroot/mozilla/editor/libeditor/base/nsEditPropertyAtomList.h,v  <--  nsEditPropertyAtomList.hnew revision: 1.4.20.1; previous revision: 1.4
done
Checking in mozilla/editor/libeditor/html/nsHTMLEditRules.cpp;
/cvsroot/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp,v  <--  nsHTMLEditRules.cpp
new revision: 1.320.12.2; previous revision: 1.320.12.1
done

Comment 16

[Adam Guthrie]

Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060621 Firefox/1.5.0.5.

Indenting the RTL paragraph works just fine now.

Comment 17

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).