Ticket #8163 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

filebrowserWindowFeatures

Reported by: Michal Malének Owned by: Saare
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

8163.patch (616 bytes) - added by Saare 3 years ago.
8163_2.patch (622 bytes) - added by Saare 3 years ago.

Change History

comment:1 Changed 3 years ago by Anna

  • Status changed from new to confirmed
  • Version set to 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 3 years ago by Saare

comment:2 Changed 3 years ago by Saare

  • Owner set to Saare
  • Status changed from confirmed to review

comment:3 Changed 3 years ago by wwalc

  • Milestone set to CKEditor 3.6.2

comment:4 Changed 3 years ago by Anna

  • Status changed from review to assigned
  • Owner changed from Saare to Anna

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 3 years ago by Saare

comment:5 Changed 3 years ago by Saare

  • Status changed from assigned to review
  • Owner changed from Anna to Saare

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 3 years ago by Anna

  • Status changed from review to review_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 3 years ago by Saare

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

Fixed with [7173].

comment:8 Changed 3 years ago by Anna

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

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