Opened 9 years ago

Last modified 8 years ago

#7173 confirmed Bug

Autogrow usability

Reported by: Garry Yao Owned by: Garry Yao
Priority: Normal Milestone:
Component: UI : Toolbar Version: 3.4
Keywords: IBM Cc: satya_minnekanti@…

Description

This's a continuation of #6408, where the "autogrow" plugin is still inconsistency in some situation:

  1. Load the editor (with "autogrow" plugin) with a long time in <pre> in order to display the h-scrollbar.
  2. Put in more stuff until the auto growing happens.
    • Actual Result: Vertical scrollbar is still there

And it even introduce an usability issue on Webkit:

  1. Load the editor (with "autogrow" plugin) in Webkit;
  2. Put in more stuff until the auto growing happens.
    • Actual Result: Host page scroll up whenever the auto growing happens.

Attachments (3)

7173.patch (2.8 KB) - added by Garry Yao 9 years ago.
7173_2.patch (3.8 KB) - added by Garry Yao 9 years ago.
ScreenShot.png (375.1 KB) - added by Dinu 9 years ago.

Download all attachments as: .zip

Change History (20)

Changed 9 years ago by Garry Yao

Attachment: 7173.patch added

comment:1 Changed 9 years ago by Garry Yao

Owner: set to Garry Yao
Status: newreview

The resizing logic is rewritten by this patch that makes it easier to adapt to cases when having scrollbar.

comment:2 Changed 9 years ago by Dinu

Doesn't work quite as well as the previous version:

  • The minimun delta of 18: not a good idea, because 1) it constantly triggers toolbars and 2) I have this use case: I use the editor to replace divs and I set the initial height of the editor to the div height. The delta for the first trigger is never 18 (usually 2-4 px accounting for table lines) so the editing almost consistently starts with scrollbars. Removing the >18px condition produces good results.
  • There is a flicker on Webkit when either the first or the last paragraph has a margin that is taller that body margin (just go to end of document, add a 50px margin paragraph, go back to wysiwyg and press down arrow a few times, height will cycle)
  • IE: it's always oversized 28px, even in standards mode (IE8) and it cycles like Chrome in IE7 even without margins
  • in Opera it works flawlessly (but the editor is broken...)
  • Mozilla: constantly 28px overtall

comment:3 Changed 9 years ago by Dinu

Please mind I'm using 0px body margins and is the best way to test, having higher margins for body can hide some flaws.

Changed 9 years ago by Garry Yao

Attachment: 7173_2.patch added

comment:4 Changed 9 years ago by Garry Yao

Polishing the patch targeting the above tests from dinu.

comment:5 Changed 9 years ago by Dinu

I don't know, but for me this just keeps growing, and growing, and growing... in Chrome, Mozilla and IE...

Changed 9 years ago by Dinu

Attachment: ScreenShot.png added

comment:6 Changed 9 years ago by Dinu

Screenshot attached... (sorry for size). It seems that:

  • The cke_contents td is not resized properly (notice the scrollbars)
  • The span that you see below in firebug just keeps growing (height is set to incresing size without reason)

comment:7 Changed 9 years ago by Garry Yao

We'd love to receive comments from a real world app, no problem, but would you mind of creating a reduced TC that's accessible for every body?

comment:8 Changed 9 years ago by Dinu

Sorry, my bad (I think). I applied the patch to 3.5.1; on the nightly build it works, and it works great! Here's the only glitch I could find:

  • make an editor with fixed width (say 600px) and 0px body margin
  • insert an element that is wide enough to not allow for vertical scrollbar (eg a table w/ 595px width)
  • as the area autogrows, it's constantly 1 srollbar higher than it should, in IE8 and FF


I think, the above can be fixed like this: if the contents' scrollWidth (or whatever quirks equivalent) is smaller than the editor width, temporarily set overflow:hidden while autogrowing. Just a thought, haven't really worked it out.

Other than that, it's excellent and works like a charm.

comment:9 Changed 8 years ago by Satya Minnekanti

Cc: satya_minnekanti@… added
Keywords: IBM added

comment:10 Changed 8 years ago by Dinu

One more problem on webkit:

  • add at least 2 paragraphs of text that are longer than 1 line (wrap) and high enough to trigger autogrow
  • editor will flicker

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.6.1
Status: reviewreview_passed

Please just rename the "resizeContents" parameter to "forContents". It took a while for me to understand it. It would be good to have its documentation in place as well.

@dinu, i was not able to reproduce the webkit issue. I found the description a bit confusing, so maybe I've missed something. Please feel free to open a dedicated ticket for it with more details. Thanks.

comment:12 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6921].

comment:13 Changed 8 years ago by Dinu

Yeap, WFM too in 11.0

comment:14 Changed 8 years ago by Dinu

(Chrome) and 5.0,5 (Safari). Sorry for the fragmented post, I tripped over submit button.

comment:15 Changed 8 years ago by Wiktor Walc

Resolution: fixed
Status: closedreopened

Due to test cases from #7984 and #8050, the change is reverted with [7056].

Last edited 8 years ago by Garry Yao (previous) (diff)

comment:16 Changed 8 years ago by Wiktor Walc

Milestone: CKEditor 3.6.1

comment:17 Changed 8 years ago by Jakub Ś

Status: reopenedconfirmed
Version: 3.53.4

Scrollbar is present on in IE6 and IE7 while other browsers show it only for a moment and then hide (I think this part is ok).

This issue is reproducible from CKEditor 3.4.

Note about Webkit duplicates #6540.

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