Opened 15 years ago

Closed 13 years ago

Last modified 13 years ago

#2988 closed New Feature (fixed)

V3: Document Properties plugin

Reported by: Artur Formella Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.6
Component: UI : Dialogs Version:
Keywords: Cc:

Description

Document Properties plugin with Select Color dialog.

Attachments (7)

2988_ref.patch (25.9 KB) - added by Sa'ar Zac Elias 13 years ago.
INCOMPLETE implementation of the plugin
2988.patch (28.5 KB) - added by Sa'ar Zac Elias 13 years ago.
2988_2.patch (27.0 KB) - added by Sa'ar Zac Elias 13 years ago.
2988_3.patch (27.8 KB) - added by Sa'ar Zac Elias 13 years ago.
2988_language_updates.patch (102.6 KB) - added by Wiktor Walc 13 years ago.
2988_4.patch (28.5 KB) - added by Sa'ar Zac Elias 13 years ago.
2988_5.patch (28.6 KB) - added by Sa'ar Zac Elias 13 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 Changed 15 years ago by Artur Formella

Owner: set to Artur Formella

comment:2 Changed 15 years ago by Frederico Caldeira Knabben

This one needs full page support, which is not yet available and it's not our priority now. Please hold on with this ticket.

comment:3 Changed 15 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.0CKEditor 3.x
Type: BugNew Feature

comment:4 Changed 15 years ago by Artur Formella

#628 is connected with ticket.

comment:5 Changed 14 years ago by Artur Formella

Owner: Artur Formella deleted

comment:6 Changed 14 years ago by Frederico Caldeira Knabben

#4588 has been marked as DUP.

comment:7 Changed 14 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.x

Milestone CKEditor 3.x deleted

comment:8 Changed 13 years ago by Garry Yao

Milestone: CKEditor 3.6

comment:9 Changed 13 years ago by Sa'ar Zac Elias

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

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988_ref.patch added

INCOMPLETE implementation of the plugin

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988.patch added

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

Status: assignedreview

comment:11 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Looks cool, but the patch breaks IE6/7, and link color options doesn't persist between mode switches - as it's quite difficult to handle them correctly, how about leave it for now?

comment:12 in reply to:  11 Changed 13 years ago by Sa'ar Zac Elias

Status: review_failedreview

Replying to garry.yao:

the patch breaks IE6/7

Another case of special usage of getAttribute, fixed.

link color options doesn't persist between mode switches - as it's quite difficult to handle them correctly, how about leave it for now?

I removed those options, and merged the remaining options in that tab with the background tab, creating the new "design" tab.
In addition, a few minor glitches had been fixed.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988_2.patch added

comment:13 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Some glitches around charset were found:

  1. Confirm an empty dialog will result in the following meta:
    <meta content="text/html; charset=" http-equiv="content-type" />
    
  2. "text/html" will be forced that show no respect of what specified by user;
  3. Other way of specifying charset is not supported:
    <meta http-equiv="charset" content="iso-8859-1">
    

The plugin should not be loaded at all in non-fullpage mode.

comment:14 Changed 13 years ago by Anna Tomanek

Just one minor note regarding the dialog window texts. I would suggest changing the following label:

Is:
Nonscrolling (Fixed) Background
Should be:
Non-scrolling (Fixed) Background

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988_3.patch added

comment:15 Changed 13 years ago by Sa'ar Zac Elias

Status: review_failedreview

All of the above suggestions, including Anna's, are covered in the 3rd patch.

Changed 13 years ago by Wiktor Walc

Attachment: 2988_language_updates.patch added

comment:16 Changed 13 years ago by Wiktor Walc

Please make sure to commit also language updates when closing this ticket (took me a while to translate everything ;)).

Not sure if storing 'previewHtml' in language makes sense, I guess "Lorem ipsum" could be reused from the Image dialog.

comment:17 in reply to:  16 Changed 13 years ago by Garry Yao

Replying to wwalc:

Please make sure to commit also language updates when closing this ticket (took me a while to translate everything ;)).

Thanks Wiktor, I didn't recognize there's still people who master every language on this planet ;)

Not sure if storing 'previewHtml' in language makes sense, I guess "Lorem ipsum" could be reused from the Image dialog.

Considering that we need a link in preview.

comment:18 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Saved content type doesn't looks like a good idea, check this:

  1. First load with xhtml content-type:
    <meta content="application/xhtml+xml; charset=utf-8" http-equiv="content-type" />
    
  2. Open dialog do nothing and close.
  3. Second load with no meta tag of content-type specified.
  4. Open dialog and specify any charset;
  5. Check output:
  • Actual Result: application/xhtml+xml specified in the first time was enforced.

comment:19 Changed 13 years ago by Garry Yao

Also we must load the plugin into fullpage sample page btw.

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988_4.patch added

comment:20 Changed 13 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:21 Changed 13 years ago by Garry Yao

Status: reviewreview_failed

Everything looks good right now except body text color is always enforced to white once open dialog, it should be instead leaving blank thus remains "inherit".

Changed 13 years ago by Sa'ar Zac Elias

Attachment: 2988_5.patch added

comment:22 Changed 13 years ago by Sa'ar Zac Elias

Status: review_failedreview

Changes will be applied after the field contents had been changed.

comment:23 Changed 13 years ago by Garry Yao

Status: reviewreview_passed

Nice done!

comment:24 Changed 13 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [6788].

comment:25 Changed 13 years ago by Sa'ar Zac Elias

Addes icon coordinates to other skinds with [6789].

Version 0, edited 13 years ago by Sa'ar Zac Elias (next)

comment:26 Changed 13 years ago by Wiktor Walc

It looks like translations were not committed... I have added them with [7028].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy