Opened 16 years ago
Closed 15 years ago
#2799 closed Bug (duplicate)
V3: The skin file make specific dialog stuff by using the global CKEDITOR.dialog
Reported by: | Frederico Caldeira Knabben | Owned by: | Garry Yao |
---|---|---|---|
Priority: | Must have (possibly next milestone) | Milestone: | CKEditor 3.0 |
Component: | General | Version: | |
Keywords: | Confirmed Review- | Cc: |
Description (last modified by )
Into the default skin.js file, we can find references to CKEDITOR.dialog.setMargins and CKEDITOR.dialog.on( 'resize' ). This is bad, because it assumes that all editor instances are using a single skin, which is false.
All settings must go inside each single editor instance. Also, events like "resize" should be fired by each instance. The event could be named "dialogResize", for example.
Attachments (2)
Change History (9)
comment:1 Changed 16 years ago by
Owner: | set to Garry Yao |
---|---|
Status: | new → assigned |
comment:2 Changed 16 years ago by
Description: | modified (diff) |
---|
Changed 16 years ago by
Attachment: | 2799.patch added |
---|
comment:3 Changed 16 years ago by
Keywords: | Review? added |
---|
comment:4 Changed 16 years ago by
Keywords: | Review- added; Review? removed |
---|
Review- because the patch doesn't work.
The margin values are there for the mouseMoveHandler()s in the dialog drag/drop and resize logic. It is used in calculating the bounds for the dialog's position and sizes, relative to the browser window.
Specifically, the margin values represent areas that logically belong to the dialog, but visually shouldn't be considered part of the dialog (e.g. dialog shadows). Those areas should be excluded from calculations of the dialog's bounding area.
Now, when we look at the patch, the margin values are saved inside the dialog object, which are under the editor object. That part is correct. However, the mouseMoveHandler()s in drag/drop and resize part of the code are still using the margin values from CKEDITOR.dialog._ - which no longer exists. The net result is the dialog bounding area calculations are wrong after the patch. e.g. If you drag a dialog around, you'll notice the bounding area now includes the shadows.
Some other minor problems with the patch:
- _source/plugins/dialog/plugin.js, line 938: This line shouldn't exist.
- _source/skins/default/skin.js, line 42, 44, 50: Wrong bracket style.
- _source/skins/default/skin.js, line 44-110: Wrong indent level (should be 1 tab instead of 2).
Changed 16 years ago by
Attachment: | 2799_2.patch added |
---|
comment:5 Changed 16 years ago by
Revised patch with the following fixes:
- Now each skin are sensitive to it's belonged editor instance.
- A init[Partname]Part method is added to skin definition to support for apply specific behaviors only to skin parts.
- Adding dialogBeforeShow and dialogBeforeResize events to editor.
- Introduce a new method: CKEDITOR.tools.capitalize.
comment:6 Changed 16 years ago by
Keywords: | Review? added; Review- removed |
---|
comment:7 Changed 15 years ago by
Keywords: | Review- added; Review? removed |
---|---|
Resolution: | → duplicate |
Status: | assigned → closed |
This one will be fixed at #3191, with a much simpler solution possibly.
Depends on: #2802.