Closed Bug 331846 Opened 15 years ago Closed 15 years ago

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

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.8.1beta1

People

(Reporter: uriber, Assigned: uriber)

Details

(4 keywords)

Attachments

(2 files, 1 obsolete file)

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.
Attached file 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.
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.
Keywords: testcase
Attached patch patch (obsolete) — Splinter Review
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.
Attachment #216404 - Flags: review?
Comment on attachment 216404 [details] [diff] [review]
patch

Timeless, are you good for reviewing this? If not, who is?
Attachment #216404 - Flags: review? → review?(timeless)
Attached patch patch v2Splinter Review
Fixes requested by timeless on IRC:
- Changed the function name to MarginPropertyAtomForIndent
- Avoid "else" after return
- Fixed some nearby indentation
Assignee: mozeditor → uriber
Attachment #216404 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #216566 - Flags: review?(timeless)
Attachment #216404 - Flags: review?(timeless)
Attachment #216566 - Flags: review?(timeless) → review+
Attachment #216566 - Flags: superreview?(bzbarsky)
Attachment #216566 - Flags: superreview?(bzbarsky) → superreview+
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
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.
BZ or timeless thoughts on safety of this patch for 1.8.1 or 1.8.0 branches?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
(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 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.
Attachment #216566 - Flags: approval1.8.0.5?
Attachment #216566 - Flags: approval-branch-1.8.1?(bzbarsky)
Sorry, that should read "Hebrew and Arabic interfaces"
Comment on attachment 216566 [details] [diff] [review]
patch v2

Looks reasonable.
Attachment #216566 - Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1+
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
Flags: blocking1.8.1?
Keywords: fixed1.8.1
Target Milestone: mozilla1.9alpha → mozilla1.8.1beta1
Comment on attachment 216566 [details] [diff] [review]
patch v2

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #216566 - Flags: approval1.8.0.5? → approval1.8.0.5+
Flags: blocking1.8.0.5? → blocking1.8.0.5+
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
Keywords: fixed1.8.0.5
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.
Status: RESOLVED → VERIFIED
Mass-assigning the new rtl keyword to RTL-related (see bug 349193).
Keywords: rtl
You need to log in before you can comment on or make changes to this bug.