Opened 10 years ago

Closed 7 years ago

#11956 closed Bug (fixed)

[IE, Chrome, Opera] Dialog box doesn't open on two words links with background.

Reported by: Artur Delura Owned by: Tomasz Jakut
Priority: Nice to have (we want to work on it) Milestone: CKEditor 4.7.0
Component: General Version: 4.0
Keywords: IE Blink Cc:

Description

  1. Open editor with following content
    Hello <a href="http://en.wikipedia.org/wiki/Neil_Armstrong">Neil Arm</a> .
    
  2. Select link Neil Arm.
  3. Change selection background color.
  4. Double click Arm word to open link dialog.

Actual result: Dialog won't open.

Please note: Double click on word Neil open dialog properly.

Change History (20)

comment:1 Changed 10 years ago by Jakub Ś

Keywords: IE Blink added
Status: newconfirmed
Version: 4.0

I confirm this issue for Blink and IE8+.

Problem can in fact be reproduced from CKEditor 3.0 but I'm setting 4.0 for it because we don't support CKE 3.x anymore.

comment:2 Changed 9 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5

comment:3 Changed 9 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:4 Changed 9 years ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:5 Changed 9 years ago by Tomasz Jakut

Status: assignedreview

It seems that clicking the link which has descendants sometimes report that descendant as a clicked element (especially if link is immediately followed by unlinked text), so I just propose to check not only the clicked element, but also its ancestors to find the clicked link. Pushed fix to branch:t/11956.

comment:6 Changed 8 years ago by Olek Nowodziński

Status: reviewreview_failed
  1. https://github.com/cksource/ckeditor-dev/commit/7e25e68a7338b1bc57fae4db0155986849b8d6aa#diff-d9e496f4ce47ad70bef0ba672713d3a1R293 isn't it exactly the same as CKEDITOR.dom.node.getAscendant? If so, please use the core method.
  1. I'd rather avoid using selection for assertions unless it is necessary (selection is slow and quirky). And since doubleclick listeners in Link Plugin communicate using evt.data object, and as long as the only thing that really matters here is whether a link has been detected, it's easier to fetch evt.data.link and check if The Link has been found, before selection kicks in. So I'd rather see it this way:
    // #11956
    'test select link with descendants on double-click': function() {
    	var bot = this.editorBot,
    		editor = bot.editor;
    
    	editor.once( 'doubleclick', function( evt ) {
    		evt.cancel();
    
    		resume( function() {
    			assert.areSame( editor.document.findOne( 'a' ), evt.data.link, 'Link selected' );
    		} );
    	} );
    
    	bot.setData( '<p>a<a href="http://bar"><span style="background:#f00;">b</span></a>c</p>', function() {
    		editor.fire( 'doubleclick', {
    			element: editor.document.findOne( 'span' )
    		} );
    
    		wait();
    	} );
    }
    
  1. Other than that, I'm cool with the solution :)

comment:7 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:8 Changed 8 years ago by Tomasz Jakut

Status: review_failedreview
  1. Yes, they're the same. I've changed code to use element.getAscendant.
  2. Test's updated.

Pushed to branch:t/11956.

comment:9 Changed 8 years ago by Olek Nowodziński

Status: reviewreview_failed

I pushed 2 minor commits to the branch. Also found an issue:

  1. Open http://ckeditor.dev/samples/.
  2. Double–click any text, which is not a link.
  3. uncaught TypeError: Cannot read property 'isReadOnly' of null(anonymous function) @ plugin.js:99 is thrown.

comment:10 Changed 8 years ago by Tomasz Jakut

Status: review_failedreview

I've fixed that issue by adding additional check if element is set, as element.getAscendant returns nothing if there is no link.

Pushed branch:t/11956.

comment:11 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:12 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:13 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:14 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:15 Changed 8 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:16 Changed 7 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

comment:17 Changed 7 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.2
Priority: NormalNice to have (we want to work on it)

Moving to the nice to have list.

comment:18 Changed 7 years ago by kkrzton

Milestone: CKEditor 4.7.0

Rebased on the current major, targeting 4.7.0.

comment:19 Changed 7 years ago by kkrzton

Status: reviewreview_passed

LGTM!

Updated manual test version tag and added elementspath plugin (so it is more visible what was selected) - aaddd57.

comment:20 Changed 7 years ago by kkrzton

Resolution: fixed
Status: review_passedclosed

Merged to major with c390920.

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