Opened 10 years ago

Closed 10 years ago

Last modified 10 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 10 years ago.
8163_2.patch (622 bytes) - added by Sa'ar Zac Elias 10 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 8163.patch added

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

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

comment:3 Changed 10 years ago by Wiktor Walc

Milestone: CKEditor 3.6.2

comment:4 Changed 10 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 10 years ago by Sa'ar Zac Elias

Attachment: 8163_2.patch added

comment:5 Changed 10 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 10 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 10 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [7173].

comment:8 Changed 10 years ago by Anna Tomanek

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

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