Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#8163 closed Bug (fixed)

filebrowserWindowFeatures

Reported by: Michal Malének Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.6.2
Component: General Version: 3.4.1
Keywords: Cc:

Description

I've noticed that there is no 'filebrowserWindowFeatures' - instead there is fileBrowserWindowFeatures (capital B) while all other "filebrowser" parameters are spelled in lower case. Is this intentional? (it took me pretty long before I realized that fileBrowserWindowFeatures was misspelled.)

Attachments (2)

8163.patch (616 bytes) - added by Sa'ar Zac Elias 6 years ago.
8163_2.patch (622 bytes) - added by Sa'ar Zac Elias 6 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by Anna Tomanek

Status: newconfirmed
Version: 3.4.1

This definitely is not intentional and what is more, API docs are even more misleading by giving the name of this configuration setting as filebrowserWindowFeatures.

As discussed with Wiktor, the code should be corrected in a way that would preserve backwards-compatibility with existing customization code that could be using the wrong spelling.

Changed 6 years ago by Sa'ar Zac Elias

Attachment: 8163.patch added

comment:2 Changed 6 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: confirmedreview

comment:3 Changed 6 years ago by Wiktor Walc

Milestone: CKEditor 3.6.2

comment:4 Changed 6 years ago by Anna Tomanek

Owner: changed from Sa'ar Zac Elias to Anna Tomanek
Status: reviewassigned

After yet another discussion with Wiktor we have decided to skip the backwards compatibility code as it does not seem that anyone used the incorrect version given that in the documentation it was spelled differently. It does not seem to make much sense to introduce redundant code then.

The new patch will correct the spelling and the API documentation.

Changed 6 years ago by Sa'ar Zac Elias

Attachment: 8163_2.patch added

comment:5 Changed 6 years ago by Sa'ar Zac Elias

Owner: changed from Anna Tomanek to Sa'ar Zac Elias
Status: assignedreview

We have no way of knowing no one uses that configuration, and we cannot just "assume" so, given the long run this configuration exists. It's very important to keep the API signature as-is. Wiktor and I agreed, then, that for now back-compat should be preserved, and removed during the v4 development.
Doc changes are to be done regardless the ticket.

comment:6 Changed 6 years ago by Anna Tomanek

Status: reviewreview_passed

If so, I think we are good to go. A minor note, though, can we change the comment to include the full and proper version of the "backward compatibility" keyword to make it easier to find later?

// TODO: V4: Remove backward compatibility (#8163).

comment:7 Changed 6 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [7173].

comment:8 Changed 6 years ago by Anna Tomanek

And the API documentation update is committed as [7174].

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