Ticket #3671 (closed Bug: fixed)

Opened 5 years ago

Last modified 5 years ago

Inserting Page break is introducing new paragraph

Reported by: garry.yao Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.0
Component: Core : Styles Version:
Keywords: Confirmed Review+ Cc:

Description (last modified by garry.yao) (diff)

Reproducing Procedures

  1. Open the replace by class example page in FF;
  2. Click on 'New Page' to empty document content;
  3. Click on 'Insert Page Break for Printing' to insert a page break;
  4. Click on the newly introduced page break fake element;
    • Expected Result: A new paragraph is created before it.
      <div style="page-break-after: always;">
      		<span style="display: none;">&nbsp;</span></div>
      
    • Actual Result: Javascript error thrown.
      <p>
         <br /></p>
      <p>
      <div style="page-break-after: always;">
      		<span style="display: none;">&nbsp;</span></div>
      </p>
      
      

Attachments

3671.patch (1.6 KB) - added by garry.yao 5 years ago.
3671_2.patch (2.5 KB) - added by garry.yao 5 years ago.
3671_3.patch (1.6 KB) - added by garry.yao 5 years ago.
3671_4.patch (2.2 KB) - added by garry.yao 5 years ago.

Change History

comment:1 Changed 5 years ago by garry.yao

  • Description modified (diff)

comment:2 Changed 5 years ago by garry.yao

  • Description modified (diff)

Oops, fix wrong description.

comment:3 Changed 5 years ago by garry.yao

This's supposed to be fixed by #3657.

Changed 5 years ago by garry.yao

comment:4 Changed 5 years ago by garry.yao

  • Keywords Review? added
  • Status changed from new to assigned

After some investigation, the fixing is not only limited to #3657, here I'm proposing a way of avoiding those fake elements been incorrectly positioned due to 'fixForBody' option of the parser.

comment:5 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review? removed

Though the patch works, the parser should have no knowledge of the fake object system, so the transformation should go into fakeobject plugin instead.

Changed 5 years ago by garry.yao

comment:6 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

Updates contained in the new patch:

1.The parser now is having a duo-pass parsing on the output html, which make fake elements have no more incorrect document position:

  • The first pass for running the htmlfilter through, without formatter;
  • The second pass for fixBody and also formatter, without any filtering; 1.The 'Page Break' plugin is now simply 'inserting' the fake element into the document, without worry about schema validation, which will be then taken care by the output parser.

comment:7 Changed 5 years ago by garry.yao

This patch's having a dependency on the latest patch from #3609 until that's get fixed.

comment:8 Changed 5 years ago by fredck

  • Keywords Review- added; Review? removed

Double parsing is not an option. A different solution needs to be found for it.

comment:9 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

So I would like to revert to the previous patch.

Changed 5 years ago by garry.yao

comment:10 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

Please add the changelog entry when committing.

comment:11 Changed 5 years ago by garry.yao

  • Keywords Review- added; Review+ removed

The correct way of retrieving real element name of fakeelement should be the following instead:

 element.attributes[ '_cke_real_element_type' ]

Changed 5 years ago by garry.yao

comment:12 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

comment:13 Changed 5 years ago by fredck

  • Keywords Review+ added; Review? removed

Oh yes, that's much better.

comment:14 Changed 5 years ago by garry.yao

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

Fixed with [3859].

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