Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9523 closed Bug (fixed)

[Webkit] ENTER scrolls up the page

Reported by: Frederico Caldeira Knabben Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 4.0
Component: General Version: 3.0
Keywords: Webkit Cc:

Description

When the editor is inside a float element, it scrolls the page up on ENTER.

I'll be attaching a test page for this.

Attachments (3)

9523_tc.html (826 bytes) - added by Frederico Caldeira Knabben 6 years ago.
Test Page for the provided TC
ckeditor.patch (651 bytes) - added by Branden Hamilton 6 years ago.
patch against 3.6.4
ckeditor-build.patch (683 bytes) - added by Branden Hamilton 6 years ago.
ckeditor release 3:2.6

Download all attachments as: .zip

Change History (22)

Changed 6 years ago by Frederico Caldeira Knabben

Attachment: 9523_tc.html added

Test Page for the provided TC

comment:1 Changed 6 years ago by Frederico Caldeira Knabben

In the attached test page, just hit ENTER at the end of the first editor contents. The page will scroll up.

Confirmed with Chrome at least.

comment:2 Changed 6 years ago by Garry Yao

Milestone: CKEditor 4.0CKEditor 4.0.1
Version: 4.0 (GitHub - master)3.0

V3 is affected as well.

comment:3 Changed 6 years ago by Jakub Ś

Resolution: duplicate
Status: newclosed

DUP of #9136

comment:4 Changed 6 years ago by Frederico Caldeira Knabben

Resolution: duplicate
Status: closedreopened

It was a mistake to make this a DUP of #9136 because it may be that the "float" issue is different than the "absolute" one. In fact, those are different TCs.

To avoid one issue blocking the other, I'm reopening this one. Luckily a fix here or there will close both issues, but we can't guarantee that beforehand.

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.0.1CKEditor 4.0
Status: reopenedconfirmed

comment:6 Changed 6 years ago by Piotrek Koszuliński

Summary: ENTER scrolls up the page[Webkit] ENTER scrolls up the page

Confirmed on Webkits only.

comment:7 Changed 6 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:8 Changed 6 years ago by Piotrek Koszuliński

If body contains float element and there's no block with e.g. overflow:hidden that would span around this float element and expand body, body has clientHeight~=0 (height of it's non floating content) and scrollHeight~=html.scollHeight. This happens only on Webkits, because on other browsers body.scrollHeight==body.clientHeight.

In this case we're incorrectly trying to scroll cursor marker into the body, which has... height~=0, so body is incorrectly scrolled. When I forced the same marker.scrollIntoParent(body) on FF result was incorrect too.

I have to find a way to distinguish the situation in which body has incorrect scrollHeight on Webkit. So far I haven't found decent solution.

comment:9 Changed 6 years ago by Garry Yao

Opened test branch t/9523 for reduced TCs.

comment:10 Changed 6 years ago by Garry Yao

Component: UI : Enter KeyGeneral
Keywords: Webkit added
Owner: changed from Piotrek Koszuliński to Garry Yao
Status: assignedreview

comment:11 Changed 6 years ago by Garry Yao

Opened dev t/9523 for review, it always check the page scroll on <html> element instead of body.

comment:12 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_passed

I have the same quick hack while testing... and I should propose it :) It looks like it doesn't break anything. Kudos Garry.

comment:13 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Changed 6 years ago by Branden Hamilton

Attachment: ckeditor.patch added

patch against 3.6.4

comment:14 Changed 6 years ago by Branden Hamilton

I was unable to cleanly apply the patch indicated in comment 13 to a 3.x version of the library. I'm not familiar with the back-porting policy for CKEditor, or if you even have one, however, please see above attached patch which applies to 3.6.4.

According to the posted roadmap for the 3.x branch http://dev.ckeditor.com/query?group=status&milestone=CKEditor+3.6.6 this bugfix is not planned for the 3.x branch, though it was originally reported to affect the 3.x version of the library.

Changed 6 years ago by Branden Hamilton

Attachment: ckeditor-build.patch added

ckeditor release 3:2.6

comment:15 Changed 6 years ago by Piotrek Koszuliński

ckeditor-build.patch looks like correctly ported. Although, I haven't checked if it works for v3.

comment:16 Changed 6 years ago by Jakub Ś

This problem was fixed in version 4. We won't be backporting bug fixes from v4 to v3.

CKEditor 4 is superior to CKEditor 3.x. It was redesigned so many bugs that were present or impossible to fix in v3 have been dealt with in v4.

CKEditor 4 is also backwards compatible with CKEditor 3. One can choose to build CKEditor with only the plugins that were available in v3.

comment:17 Changed 6 years ago by Branden Hamilton

@ j.swiderski Thanks for the followup.

Based on your latest post, it sounds like CKSource's official stance is to deprecate the use of the 3.x branch, or at least only provide security fixes. While I do not think this is an unfounded approach, the 4.x branch is a significant improvement, my explicit use case necessitates the continued use of the 3.x branch in the near term.

Specifically, consumers of your integration solutions are still using the 2.x (CKEditor Enterprise) or 3.x (http://drupal.org/project/ckeditor) branches. While I understand it will be largely a community effort to develop the necessary Drupal module (ie. http://drupal.org/node/1748268), or Joomla extension that implements the 4.x version, neither project appears to have caught up with CKSource to date.

As such, I simply wanted to point out that for some, CKSource's official stance would essentially leave many out in the cold. Some, like myself, can get around this by utilizing patches like the one I've posted here, but I suspect most do not have the means to do so.

Speaking as a Drupal developer, we typically do not maintain "upstream" library patches within our own community ticket system, and are encouraged to work with the source library's maintainers to provide patches there where they might benefit not only our community, but anyone using said library.

Again, I am not taking the stance that CKSource should indefinitely maintain backwards support for previous branches, however, a portion of your user base is currently dependent upon the older version. The patch I included was merely an attempt to alleviate some of that burden until our community has had the chance to catch-up and support the 4.x branch.

Thanks for your time and consideration

Last edited 6 years ago by Branden Hamilton (previous) (diff)

comment:18 Changed 6 years ago by Jakub Ś

Hi,

Another official statement is that currently our PHP developers and main Drupal developer is working very hard to prepare these integrations ASAP. We think it(perhaps they) should be ready within a month.
We had to make a choice and decided to take step by step approach, releasing our main product first and then others, also one by one.
All I can say is, please give us some time and new Drupal/Joomla version will be released.

deprecate the use of the 3.x branch

We can't do that. We will be definitely encouraging our users to switch to v4 but v3 will not be deprecated (perhaps in some distant future).
Like I have written before v4 is better and backwards compatible.
I’m not just talking about new features but new approach. Some issues that were "won’t fixes" are now not a problem.
As for backwards compatibility, sure this is not the same code and some advanced plugins will have to be adjusted to new version (E.g. I'm doing this right now with my autosave plugin) but basic usage and configuration (which is done in most cases) should not give users/devs any problems.

comment:19 Changed 6 years ago by Branden Hamilton

@j.swiderski,

Thanks for the transparency. I probably misspoke by using the term 'deprecate'. I was attempting to speak to the availability of bug fixes for v3, and not necessarily to the availability of the product itself. Nevertheless, I'm greatly encouraged to hear you're still committed to releasing updated versions of your integration solutions. Additionally, thanks for the clarification concerning v4 and v3 backwards compatibility.

We think it(perhaps they) should be ready within a month.

Given how hard CKSource is working in this area, I do not think the concerns I raised will have as great an affect as I previously thought. Thanks again.

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