Changes between Initial Version and Version 1 of Ticket #9829, comment 12
- Timestamp:
- Jan 11, 2013, 12:50:45 PM (11 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #9829, comment 12
initial v1 4 4 These methods aren't part of unit tests. {{{DataFilter#test()}}} is a public method which plugins use to check if some content is allowed. It accepts the string based format accepted by allowedContent configuration and basic {{{CKEDITOR.style}}} objects. 5 5 6 [[BR]] 7 6 8 > On second thought and on grepping the code base, it seems like this is necessary to figure out whether a button should be allowed to appear at all in the toolbar, i.e. to check whether it doesn't conflict with the `dataFilter` configuration? 7 9 8 10 Exactly :). BTW. `git diff master...t/9892` is convenient way of checking what's changed since this branch diverged from master. 11 12 [[BR]] 13 9 14 10 15 > … … 13 18 14 19 Hmm. You're right. I asked for feedback, so at least basic comments for the most important stuff would be great. I'll add them asap. 20 21 [[BR]] 15 22 16 23 > … … 21 28 "blue" doesn't look like a tag for me, because semicolons are used for rules separation. But you may be right that grouping classes by "()" will be clearer for people who hasn't spent hours starring at allowedContent like me :D. Let's see what others think. 22 29 30 [[BR]] 31 23 32 >Even better would be if it would resemble a well-known syntax: inspired by regular expressions syntax, it could become this: 24 33 > {{{ … … 28 37 > 29 38 30 I omitted pipes leaving them for values separator. If we'll decide to implement simple values checking then it would be great to have: {{{img[attr=val1|val2|val3, attr2=...]}}}. Also in regexp like fashion. Comma also has one advantage over pipe for being an attributes, classes and styles separator - in natural language we used to put space after it, what makes text more readable. And this is now supported by {{{dataFilter}}}. However, such a space would change regexp meaning. 39 I omitted pipes leaving them for values separator. If we'll decide to implement simple values checking then it would be great to have: {{{img[attr=val1|val2|val3, attr2=...]}}}. Also in regexp like fashion. Comma also has one advantage over pipe for being an attributes, classes and styles separator - in natural language we used to put space after it, what makes text more readable. And this is now supported by {{{dataFilter}}}. However, such a space after pipe would change regexp meaning. 40 41 [[BR]] 31 42 32 43 > … … 41 52 Opposite way is presented by the 'editor 3' (4th one :). Buttons are explicitly removed from toolbar by 'removeButtons' and 'removePlugins' configs and their features are magically stripped out from content of the editor. See that there are no image, no anchors, no strike-through *in the content* of editor 3. 42 53 54 [[BR]] 55 43 56 > > justify - here I had problem how to verify if these buttons are allowed - I'm checking only for 'p{text-align}' 44 57 > That makes sense, but I just wanted to make sure that one thing will become possible eventually: in Drupal, we don't ever want `style` attributes in our HTML content. Everything should be CSS-overridable. What we want instead for alignment, is the use of classes. IIRC, it's possible to have the alignment buttons set classes on elements, so that's a non-issue, but I want to make sure that it's also possible that we can also configure `allowedContent` accordingly. 45 58 46 59 :) I was wondering if anyone will notice that I forgot about this. You're right that [http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-justifyClasses it is possible to force justify to apply classes rather than inline styles]. Plugin should take that into consideration and basing on configuration allow styles or classes. It's of course doable, but I forgot about that :). 60 61 [[BR]] 47 62 48 63 > … … 55 70 In any case - updating dialogs will require some amount of work, but it would be pretty easy work - each config dependent field should be displayed/enabled if it is validated by {{{dataFilter.test()}}}. We need to check if we can simplify this task even more by some magic. 56 71 72 [[BR]] 73 57 74 > 58 75 > > I hope that you liked my child :). Feedback is greatly appreciated. Especially regarding API.
