Ticket #6728 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

BIDI: BiDi buttons do not work on a list that is inside a blockquote

Reported by: james c Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.5.1
Component: Core : BiDi Version: 3.4.2
Keywords: IBM Cc: damo, joek, satya

Description

Steps to reproduce the defect:

  1. Open the Ajax sample.
  2. Create a Numbered List.
  3. Select the Numbered List & click the RTL icon.
  4. Select the list & click Blockquote.
  5. Click on the LTR icon.

Expected: LTR language direction is applied to the Numbered List. The list appears on the left of the editor. The List is inside a Blockquote.

Actual: Nothing happens. The list remains RTL.

Attachments

6728.patch (1.1 KB) - added by tobiasz.cudnik 3 years ago.
6728_2.patch (544 bytes) - added by garry.yao 3 years ago.
6728_3.patch (1.2 KB) - added by tobiasz.cudnik 3 years ago.
6728_4.patch (684 bytes) - added by tobiasz.cudnik 3 years ago.
6728_5.patch (1.5 KB) - added by garry.yao 3 years ago.
6728_6.patch (2.7 KB) - added by garry.yao 3 years ago.
6728_7.patch (4.9 KB) - added by garry.yao 3 years ago.

Change History

comment:1 Changed 3 years ago by garry.yao

  • Status changed from new to confirmed
  • Milestone set to CKEditor 3.5.1

comment:2 Changed 3 years ago by tobiasz.cudnik

  • Owner set to tobiasz.cudnik
  • Status changed from confirmed to assigned

Changed 3 years ago by tobiasz.cudnik

comment:3 Changed 3 years ago by tobiasz.cudnik

  • Status changed from assigned to review

Changed 3 years ago by garry.yao

comment:4 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

I think we simply apply the direction on the inner most block container (guard elements), in this case when user would like to instead apply it on an outer container, e.g. in this case he want to apply it to "blockquote", he can achieve that with the elements path bar (by first click on the blockquote button).

comment:5 follow-up: ↓ 8 Changed 3 years ago by tobiasz.cudnik

  • Status changed from review_failed to review

This is yet another case of the "apply to parent when all children selected" from BiDi cases, which is a feature.

Check this TC:

<blockquote>
	<p>
		a</p>
	<ul>
		[<li>
			1</li>
		<li>
			2</li>
		<li>
			3</li>]
	</ul>
	<p>
		b</p>
</blockquote>

comment:6 Changed 3 years ago by Saare

Please update the patch.

Changed 3 years ago by tobiasz.cudnik

comment:7 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

6728_3.patch doesn't work for the original TC.

comment:8 in reply to: ↑ 5 Changed 3 years ago by garry.yao

Replying to tobiasz.cudnik:

This is yet another case of the "apply to parent when all children selected" from BiDi cases, which is a feature.

Check this TC:

<blockquote>
	<p>
		a</p>
	<ul>
		[<li>
			1</li>
		<li>
			2</li>
		<li>
			3</li>]
	</ul>
	<p>
		b</p>
</blockquote>

I don't get the point, how does 6728_2.patch fail for the patch?

Changed 3 years ago by tobiasz.cudnik

comment:9 Changed 3 years ago by tobiasz.cudnik

  • Status changed from review_failed to review

Please ignore my previous TC. 6728_2.patch also doesn't work with current trunk, but even if it would we can't accept such change because of this TC.

I'm sending new approach which fixes the problem with a quite small change.

comment:10 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

6728_4.patch indeed covers a bug, while having no benefit to the ticket bug itself, I still insist on applying dir on the innermost block limit, as that's the easiest way to avoid further bugs caused by differential direction on nested block limits.

Regard your cited TC, I think we could safely remove "tr" from guard elements.

Changed 3 years ago by garry.yao

comment:11 Changed 3 years ago by garry.yao

  • Owner changed from tobiasz.cudnik to garry.yao
  • Status changed from review_failed to review

comment:12 Changed 3 years ago by Saare

  • Status changed from review to review_failed

Using the following content and selection:

<table border="1" cellpadding="1" cellspacing="1" style="width: 500px;">
	<tbody>
		<tr>
			[<td>
				aa</td>
			<td>
				bb</td>]
		</tr>
		<tr>
			<td>
				&nbsp;</td>
			<td>
				&nbsp;</td>
		</tr>
		<tr>
			<td>
				&nbsp;</td>
			<td>
				&nbsp;</td>
		</tr>
	</tbody>
</table>

Click on RTL icon. Note that only the first cell of the two selected is in RTL.

comment:13 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

@Saar, the above cited TC is a bug of the domiterator reported with [6324], new patch covers it.

Changed 3 years ago by garry.yao

comment:14 Changed 3 years ago by Saare

  • Status changed from review to review_failed
  • Start with the following content and selection:
    <table border="1" cellpadding="1" cellspacing="1" style="width: 500px;">
    	<tbody>
    		<tr>
    			[<td dir="rtl">
    				a</td>
    			<td dir="rtl">
    				b</td>
    		</tr>
    		<tr>
    			<td>
    				&nbsp;</td>
    			<td>
    				&nbsp;</td>]
    		</tr>
    	</tbody>
    </table>
    
  • Click on LTR button.

Note that the table is corrupted.

Changed 3 years ago by garry.yao

comment:15 Changed 3 years ago by garry.yao

  • Status changed from review_failed to review

Nice catch, but yet another bug from domiterator.

comment:16 Changed 3 years ago by Saare

  • Status changed from review to review_passed

comment:17 Changed 3 years ago by garry.yao

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [6333].

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy