Opened 13 years ago

Closed 13 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: Andrew, Vinícius 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 13 years ago.
6390_2.patch (2.8 KB) - added by Garry Yao 13 years ago.
6390_3.patch (6.6 KB) - added by Garry Yao 13 years ago.
6390_4.patch (6.3 KB) - added by Garry Yao 13 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 13 years ago by Krzysztof Studnik

Component: GeneralUI : Dialogs
Status: newconfirmed

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 13 years ago by Garry Yao

Milestone: CKEditor 3.5.1
Version: 3.4.13.0

comment:3 Changed 13 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: confirmedassigned

Changed 13 years ago by Tobiasz Cudnik

Attachment: 6390.patch added

comment:4 Changed 13 years ago by Tobiasz Cudnik

Status: assignedreview

comment:5 Changed 13 years ago by Alfonso Martínez de Lizarrondo

Cc: Andrew added

#6848 has been marked as dup

comment:6 Changed 13 years ago by Alfonso Martínez de Lizarrondo

Cc: Vinícius Krolow added

#6231 has been marked as dup

comment:7 Changed 13 years ago by Andrew

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 13 years ago by Alfonso Martínez de Lizarrondo

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 13 years ago by Garry Yao

Attachment: 6390_2.patch added

comment:9 Changed 13 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 13 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 13 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 13 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 13 years ago by Garry Yao

Attachment: 6390_3.patch added

comment:13 Changed 13 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 13 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 13 years ago by Dinu

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

comment:16 Changed 13 years ago by Tobiasz Cudnik

Status: reviewreview_failed

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

comment:17 Changed 13 years ago by David

Cc: david@… added

Changed 13 years ago by Garry Yao

Attachment: 6390_4.patch added

comment:18 Changed 13 years ago by Garry Yao

Status: review_failedreview

comment:19 Changed 13 years ago by David

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 13 years ago by Sa'ar Zac Elias

Status: reviewreview_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 13 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [6303].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy