Opened 10 years ago

Closed 10 years ago

#3356 closed Bug (fixed)

Ident problem with nested block

Reported by: Garry Yao Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

Reproducing Procedures

  1. Open the replace by class example page;
  2. Make the following content along with selection:
            ^<div>
                text
                <p>
                    paragraph
                </p>
            </div>^
    
  3. Click on 'Increase Indent' command;
    • Expected Result:
              <div style="margin-left: 40px;">
                  <p>
                      text
                  </p>
                  <p>
                      paragraph
                  </p>
              </div>
      
    • Actual Result: Javascript error thrown.
              <div>
                  <p>
                      text
                  </p>
                  <p>
                      paragraph
                  </p>
              </div>
      

Attachments (1)

3356.patch (860 bytes) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 10 years ago by Garry Yao

Owner: set to Garry Yao
Status: newassigned

comment:2 Changed 10 years ago by Garry Yao

Summary: Ident problem with pseudo blockIdent problem with nested block

The problem happened on all nested blocks, the input of this TC could be simplified to:

		^<div>
					<p>
						nested
					</p>
		</div>^

Changed 10 years ago by Garry Yao

Attachment: 3356.patch added

comment:3 Changed 10 years ago by Garry Yao

Keywords: Review? added

The substree examination logic will prevent nested block from been indented which happened always.

comment:4 Changed 10 years ago by Garry Yao

I'll have Martin to help reviewing this ticket to see if the removed lines has impact on other tickets.

comment:5 Changed 10 years ago by Martin Kou

Keywords: Review+ added; Review? removed

The original commonParents code, ported from v2, was supposed to handle case like this:

<div id="div1">
	fdasjkfldasjkflsdadsa
	<div id="div2">
		jdaslfjlsa
		<div id="div3">
			fsdja
			<div>
				fdsjalkfjasdk
				<div>
					fdjaklfjasd</div>
				lsdkflsadl</div>
			fldjsklfjsadfsd</div>
		fjdjdas </div>
	fdjaskfldajkfsd </div>

The problem at that time was that the dom iterator would report div1 and div2 and div3 as separate paragraphs. So indenting with div1 selected would mean div1 is indented 40px, div2 is indented 40px + 40px = 80px and div 3 is indented 40px * 3 = 120px.

Now this is no longer happening with the dom iterator, as the dom iterator can now correctly separate the things between div1 and div2, and div2 and div3 into new paragraphs. So it would make sense to ignore the common parent node and indent them all. So it's correct to remove the common parent checking logic.

Review+

comment:6 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [3441]. Click here for more info about our SVN system.

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