Opened 2 years ago

Closed 2 years ago

#14747 closed New Feature (fixed)

The enhanced image caption does not support the link target attribute.

Reported by: ahmad Owned by: Tade0
Priority: Normal Milestone: CKEditor 4.5.11
Component: UI : Widgets Version: 4.3
Keywords: Cc:

Description

Steps to reproduce

1.Click on the enhanced image (image2) in the editor and add a url to the image and select the captioned image checkbox.

  1. select the caption text and click the link icon in the editor.
  2. in the link dialog you will notice that the target tab is disabled

Expected result

the expected result should be that the caption should also support the link target attribute.

you can reproduce this on the demo page.

Attachments (1)

Screen Shot 2016-08-19 at 15.04.15.png (22.2 KB) - added by kkrzton 2 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 2 years ago by ahmad

Component: GeneralUI : Widgets

comment:2 Changed 2 years ago by ahmad

Keywords: Enhanced image plugin added

comment:3 Changed 2 years ago by ahmad

Keywords: image2 added; Enhanced image plugin removed

comment:4 Changed 2 years ago by Jakub Ś

Keywords: image2 removed
Status: newconfirmed
Summary: the enhanced image caption does not support the link target attribute.The enhanced image caption does not support the link target attribute.
Type: BugNew Feature
Version: 4.5.10 (GitHub - master)4.3

This is not a bug. This is a feature.

It is enough to add target attribute to caption's ACF https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/image2/plugin.js#L289

comment:5 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11

comment:6 Changed 2 years ago by ahmad

I added like this

allowedContent: 'br em strong sub sup u s target; a[!href]; a[target]'
allowedContent: 'br em strong sub sup u s target; a[!href]'
allowedContent: 'br em strong sub sup u s; a[!href]; a[!target]'

but none of it worked. Can you please guide me how to add the target attribute?

Last edited 2 years ago by ahmad (previous) (diff)

comment:8 Changed 2 years ago by Tade0

Owner: set to Tade0
Status: confirmedassigned

comment:9 Changed 2 years ago by Tade0

Status: assignedreview

Changed the filtering so that now link targets are allowed in the caption.

Changes pushed to branch:t/14747.

comment:10 Changed 2 years ago by kkrzton

Status: reviewreview_failed

The fix is fine.

Unfortunately, all IE browsers (IE8-IE11, Edge) has different attributes order and the added test fails (see attached screenshot).

Changed 2 years ago by kkrzton

comment:11 Changed 2 years ago by Marek Lewandowski

Just to chime in, assert.isInnerHtmlMatching will be your friend.

comment:12 Changed 2 years ago by Tade0

Status: review_failedreview

Corrected the error, rebased with master, changes pushed to branch:t/14747.

comment:13 Changed 2 years ago by kkrzton

Status: reviewreview_passed

LGTM, R+.

comment:14 Changed 2 years ago by kkrzton

Resolution: fixed
Status: review_passedclosed

Merged to master with 2f98a20.

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