Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#7429 closed Bug (fixed)

CKEditor doesn't scroll to show matched words when searching or replacing

Reported by: Jakub Ś Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: General Version: 3.0.1
Keywords: IBM Cc: satya_minnekanti@…, Lynne Kues

Description (last modified by Wiktor Walc)

  1. Paste the following code
    <ul>
    	<li>
    		Copy form word</li>
    	<li>
    		copy from word 3 level lists</li>
    	<li>
    		klawiszeTemty kt&oacute;re powtarzają się i warto najpierw zrobić search.</li>
    </ul>
    <p>
    	<strong>Temty kt&oacute;re powtarzają się i warto najpierw zrobić search.</strong></p>
    <p>
    	&nbsp;</p>
    <ul style="list-style-type: decimal;">
    	<li>
    		Copy form word</li>
    	<li>
    		copy from word 3 level lists</li>
    	<li>
    		klawisze
    		<div>
    			<strong>Temty kt&oacute;re powtarzają się i warto najpierw zrobić search.</strong></div>
    		<div>
    			&nbsp;</div>
    	</li>
    	<li>
    		Copy form word</li>
    	<li>
    		copy from word 3 level lists</li>
    	<li>
    		klawiszeTemty kt&oacute;re powtarzają się i warto najpierw zrobić search.<br />
    		<br />
    		&bull; Copy form word<br />
    		&bull; copy from word 3 level lists<br />
    		&bull; klawisze<br />
    		<p>
    			&nbsp;</p>
    	</li>
    </ul>
    
  1. Go to wysiwyg mode.
  2. Search for word "copy"
  3. You will se that editor doesn't scroll to show matching words.

This dopesn't work under FF3.6 in WIN7 and XP since CKEditor version 3.0.1

In CKEditor version 3.0 "cycle search" doesn't work at all. When editor reaches the end of the document it doesn't come back up and displays the dialog - no occurrences found.

NOW THE FUN PART: The text I have pasted is big enough for scrolling, but try to double the text by pasting it once more. Now everything works. Magic :)

NOTE : I HAVEN'T TESTED IT ON FF4 YET.

Attachments (1)

7429.patch (2.5 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 8 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added

comment:3 Changed 8 years ago by Jakub Ś

Keywords: FF3.6 removed

The issue concerns all Browsers

comment:4 Changed 8 years ago by Jakub Ś

Description: modified (diff)

comment:5 Changed 8 years ago by Jakub Ś

Description: modified (diff)

comment:6 Changed 8 years ago by Wiktor Walc

Description: modified (diff)

comment:7 Changed 8 years ago by Jakub Ś

Cc: Lynne Kues added
Keywords: IBM added

comment:8 Changed 8 years ago by Jakub Ś

This is just a wild guess, but maybe toolbar height gets somehow included in the editing area and that is why sample text with particular height is not scrolled properly.

Changed 8 years ago by Garry Yao

Attachment: 7429.patch added

comment:9 Changed 8 years ago by Garry Yao

Owner: set to Garry Yao
Status: confirmedreview

Propose a rewrite of element::scrollIntoView, deprecate "alignTop" param as it's not actually in effect.

comment:10 Changed 8 years ago by Jakub Ś

An extension to this ticked based on forum post http://cksource.com/forums/viewtopic.php?f=11&t=22899&p=58594#p58594. This part is reproducible in all versions of IE from CKE 3.0.2.

  1. Paste the following code or use the one attached to original ticked:
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    <p>
    	This is some <strong>sample text</strong>. You are using <a href="http://ckeditor.com/">CKEditor</a>.</p>
    
  2. Put the cursor in the first line
  3. PageDown all the way
  4. Now try to PageUp all the way.

Result: Cursor will move to the first line but CKE won't scroll up just like in case of find/replace.

From CKE 3.0 to CKE 3.0.1 editor didn't scroll both ways. From CKE 3.0.2 it doesn't scroll up.

comment:11 Changed 7 years ago by Teresa Monahan

The problem here is the following code:

// offset value might be out of range(nagative), fix it(#3692).
offset = offset < 0 ? 0 : offset;

// Scroll the window to the desired position, if not already visible(#3795).
var currentScroll = win.getScrollPosition().y;
if ( offset > currentScroll || offset < currentScroll - winHeight )
	win.$.scrollTo( 0, offset );

The offset is reset to zero if it is a negative value BEFORE the conditional that determines whether the offset is in the current viewport or not. It should not be reset until AFTER this check has occurred.

The original fix for #3692 did this correctly, so I'm not sure why or when this got changed. In any case changing the code to the following resolves the issue:

// Scroll the window to the desired position, if not already visible(#3795).
var currentScroll = win.getScrollPosition().y;
if ( offset > currentScroll || offset < currentScroll - winHeight )
	win.$.scrollTo( 0, offset > 0 ? offset : 0  );		// offset value might be out of range(nagative), fix it(#3692).

This is a high priority issue for us. Can it be fixed in the 3.6.3 release please?

comment:12 Changed 7 years ago by Garry Yao

Status: reviewassigned

Cancel the review as of comment:4:ticket:7946.

comment:13 Changed 7 years ago by Jakub Ś

#8625 was marked as duplicate

Comment from this ticket looks similar to comment:10

To reproduce:

  1. Go to replacebycode sample
  2. Press Ctrl+A , and Delete (clear the contents)
  3. Type 'test'
  4. Put the cursor at the beginning of the word ^test
  5. Start pressing enter, until scrollbar appears. (It should be about 8 Enter key presses)
  6. Press pageUp

Result: Cursor jumps up but the editor does not scroll to show it.

Issue has been reproducible in all IE's from CKEditor 3.0

NOTE:If you press Enter key many times (let's say about 20 or 30) pageUp will work

comment:14 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Ticket is fixed by #7946, tc from comment 10 is moved #8821.

comment:15 Changed 7 years ago by Garry Yao

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