Opened 6 years ago

Closed 5 years ago

#6390 closed Bug (fixed)

Multiple clicks on toolbar buttons cause dialogs to fail

Reported by: dinu Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.5.1
Component: UI : Dialogs Version: 3.0
Keywords: Cc: joomlafck, krolow, david@…

Description

If the server is slow loading the dialog window, we have problems:

  • The screen is not blanked, there is no indication to the user that something is going on
  • The toolbar buttons are still clickable
  • If a second click is made before the screen is blanked, when the dialog opens it has the blank on top and renders the whole page useless

Possible fix: blank the screen before dialog load?

Also manifests when double-doubleclicking images etc.

Attachments (4)

6390.patch (749 bytes) - added by tobiasz.cudnik 6 years ago.
6390_2.patch (2.8 KB) - added by garry.yao 6 years ago.
6390_3.patch (6.6 KB) - added by garry.yao 6 years ago.
6390_4.patch (6.3 KB) - added by garry.yao 5 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 6 years ago by krst

  • Component changed from General to UI : Dialogs
  • Status changed from new to confirmed

Confirmed for CKEditor demo (on slow connection), first load of CKEditor only

  • open browser
  • go to demo
  • when editor is loaded click "insert image" button, and before dialog window appear click again.
  • displayed dialog window is disabled, mouse cursor is busy. page must be reloaded to re-enable CKE.

I cannot reproduce it on localhost, and when editor was loaded once.

comment:2 Changed 6 years ago by garry.yao

  • Milestone set to CKEditor 3.5.1
  • Version changed from 3.4.1 to 3.0

comment:3 Changed 6 years ago by tobiasz.cudnik

  • Owner set to tobiasz.cudnik
  • Status changed from confirmed to assigned

Changed 6 years ago by tobiasz.cudnik

comment:4 Changed 6 years ago by tobiasz.cudnik

  • Status changed from assigned to review

comment:5 Changed 6 years ago by alfonsoml

  • Cc joomlafck added

#6848 has been marked as dup

comment:6 Changed 6 years ago by alfonsoml

  • Cc krolow added

#6231 has been marked as dup

comment:7 Changed 6 years ago by joomlafck

I Know this issue has been set to fix for 3.5.1 but I can't understand why it cannot go in the 3.4.3 or even the 3.5 release as it is just adding a simpile flag to the CKEDITOR.dialog show and hide method to stop the showCover function been called twice.

I would understand if the issue was not so easy to replicate in Firefox as in the browsers such as IE, Safari and chrome. However, in one of the most used and favourite browser Firefox this issue can be replicated by just simply double clicking on a toolbar button to lanch a dialog. In Firefox this issue seems to be independent of internet connection as it always seems to occur.

Regards

comment:8 Changed 6 years ago by alfonsoml

3.4.3 and 3.5 are almost finished and in the testing phase.

As a developer you should understand that the final cut for a version must be set at some point and if "just this little fix" is added, then another one can be added also, and suddenly you have to test everything over, and a new bug pops up and a little delay is added to fix that...

Besides that, I don't think that it's such an obvious bug if it has existed since 3.0 and was reported just 4 months ago

Changed 6 years ago by garry.yao

comment:9 Changed 6 years ago by garry.yao

  • Owner changed from tobiasz.cudnik to garry.yao

The lock should start when loading the dialog via network instead of when showing up the dialog (takes a shorter time).

comment:10 Changed 6 years ago by tobiasz.cudnik

I'm not sure if we need such type of a lock, which locks all dialogs when any script is loaded. Simple one in onShow has no drawbacks and keeps a shorter lock, which is better with same results.

comment:11 Changed 6 years ago by dinu

I think

  • The blanking/locking should happen at the moment the button is clicked
  • The blanking/locking should happen in editor or plugin code, not in dialog code

Because:

  • It shows the user something is actually going on when he presses the button; some users get desperate when they click something and nothing happens and go on a clicking frenzy with unpredictable results
  • the user should not be able to type or otherwise change the document after the click, I see some plugins assume this
  • some people (like me :D) have plugins w/ dialogs that don't use the standard dialog/plugin.js (hence use their own locking/blanking scheme), it would be nice if there was no racing condition here.

comment:12 Changed 6 years ago by garry.yao

@Tobias, dialog showing occupies the browser, so there's no need for a lock, a lock is only for an async operation (dialog loading here).

@dinu, you always pack your comments ;) but make lot of senses, actually once a dialog button is clicked, the editor should be completely locked.

Changed 6 years ago by garry.yao

comment:13 Changed 6 years ago by garry.yao

The proposed patch lock screen (with focus moved to cover) before loading the dialog, not a perfect solution, but should be enough.

comment:14 Changed 6 years ago by garry.yao

some people (like me :D) have plugins w/ dialogs that don't use the standard dialog/plugin.js (hence use their own locking/blanking scheme), it would be nice if there was no racing condition here.

I'd sorry to say that dialog/plugin.js is NOT a file for customization.

comment:15 Changed 6 years ago by dinu

No customization, I skip it altogether :) Never load for my dialogs.

comment:16 Changed 6 years ago by tobiasz.cudnik

  • Status changed from review to review_failed

Using 6390_3.patch, a second dialog opening doesn't activate the cover.

comment:17 Changed 5 years ago by tech.lawson

  • Cc david@… added

Changed 5 years ago by garry.yao

comment:18 Changed 5 years ago by garry.yao

  • Status changed from review_failed to review

comment:19 follow-up: Changed 5 years ago by tech.lawson

The 6390_4.patch doesn't work for me - if the dialog has not been cached before and using an external server (i.e. not localhost), a double click still creates a cover over the dialog

comment:20 in reply to: ↑ 19 Changed 5 years ago by Saare

  • Status changed from review to review_passed

Replying to tech.lawson:

The 6390_4.patch doesn't work for me - if the dialog has not been cached before and using an external server (i.e. not localhost), a double click still creates a cover over the dialog

I'm not having this one with Firefox Throttle, so it's an R+. If you have this problem in the nightly build in the day after the commit, please let us know.

comment:21 Changed 5 years ago by garry.yao

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

Fixed with [6303].

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