Opened 7 years ago

Closed 7 years ago

#9251 closed Bug (fixed)

Magicline goes wrong with enterMode=BR

Reported by: Garry Yao Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.0
Component: General Version: 4.0
Keywords: Cc: Olek Nowodziński

Description

The magic line feature doesn't work well in enterMode BR in the following sense:

  • Wrong triggering when encountered pseudo block
  • Inserted paragraph instead of BR to establish line height

TC1:

<!-- insert new line here -->
<ol>
	<li>bar</li>
</ol>

The above insertion should result in: For non-IE:

^<br />
<ol>
	<li>bar</li>
</ol>

For IE:

^&nbsp;
<ol>
	<li>bar</li>
</ol>

TC2:

foo
<ol>
	<li>^bar</li>
</ol>

No trigger for the above case.

Change History (6)

comment:1 Changed 7 years ago by Olek Nowodziński

Status: newassigned

Created dev&test branches for this ticket.

comment:2 Changed 7 years ago by Olek Nowodziński

Status: assignedreview

Dev and test branches (t/9251) bring a lot of changes and improvements:

  • Totally revisited all the types of triggers for robustness:
    • <br> elements are handled correctly.
    • Comments and text nodes are also considered differently.
    • Different enterMode setups are handled by the plugin.
  • Revisited http://ckeditor4.t/ckeditor/plugins/magicline/samples/sample.html to make debugging easier.
    • The look has changed.
    • Explained different samples.
    • Accessing console.log debug is possible by ?debug parameter. It is disabled by default to avoid biased time measurements.
  • Totally revisited magicline tests:
    • All the cases use the same, simple testEditor call.
    • Added tests for enter modes.
    • Changed tests for triggers to have a clear view on the methods.
    • Used separate tests for each type of trigger.
  • Documented important methods and elements with ASCI doc string. Now they're straightforward for people.
  • Fixed lots of typos, doc strings, ambiguities. Cleaned up the code.

Have a nice testing! ;)

comment:3 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The following result applied on the enterBR instance of magicline plugin sample, in IE9, with the following source loaded:

<table>
	<tr>
		<td>cell</td>
	</tr>
</table>
<br />
<hr style="margin: 30px 0"/>
<ol>
	<li>item</li>
</ol>

  1. Click inside of the first table cell (but not anywhere else)
  • Actual: Magicline is not showing wherever mouse moves to
  1. Blur the editor instance and click again inside of the list item;
  • Actual: Magicline is shown.

  1. Click to insert magic line anywhere
  • Actual: Object doesn't support property or method 'scrollIntoView'

  1. Focus the editor, mouse move to the bottom of table, or upon HR element (when "move" cursor type is shown)
  • Actual: Magicline is displayed, which should not be there.

comment:4 in reply to:  3 Changed 7 years ago by Olek Nowodziński

Status: review_failedreview

Thanks for your feedback!

Replying to garry.yao:

  1. Click inside of the first table cell (but not anywhere else)
  • Actual: Magicline is not showing wherever mouse moves to
  1. Blur the editor instance and click again inside of the list item;
  • Actual: Magicline is shown.

This isn't actually a magicline-related thing. I created a ticket #9283 as it can be reproduced anywhere. Something is really broken with focusManager.hasFocus (and derivatives) in IE.


  1. Click to insert magic line anywhere
  • Actual: Object doesn't support property or method 'scrollIntoView'

I fixed this with 0812c40.


  1. Focus the editor, mouse move to the bottom of table, or upon HR element (when "move" cursor type is shown)
  • Actual: Magicline is displayed, which should not be there.

Fixed with e231b78.


Additionally, I extended tests with a number of similar cases to make the plugin bulletproof.

comment:5 Changed 7 years ago by Garry Yao

Status: reviewreview_passed

comment:6 Changed 7 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

Fixed with d90b0b4 (dev), 8cbd2d2 (tests). Masterized.

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