Opened 5 years ago

Closed 5 years ago

#11359 closed Bug (fixed)

Link dialog shows anchors from entire document in inline and div-based editors

Reported by: Piotrek Koszuliński Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.3.3
Component: UI : Dialogs Version: 3.6.6
Keywords: Cc:

Description

  1. Add <a name="xxx" id="xxx"></a> somewhere in the non-editable part of divarea and inlineall samples.
  2. See that link dialog shows this anchor in the anchors drop down.

Only anchors from the editor's content should be displayed. This behaviour may be configurable, because in some cases, especially in inline editors, it may make sense to link to the surrounding anchors.

Change History (17)

comment:1 Changed 5 years ago by Piotrek Koszuliński

Status: newconfirmed

comment:2 Changed 5 years ago by Piotrek Koszuliński

Status: confirmedpending

It looks that we need to discuss this first. Some of us said that in inline editors all anchors should be shown.

comment:3 Changed 5 years ago by Marek Lewandowski

Definitely we should take care of that, since it messes (especialy id-based dropdown) with anchors created used especially for layout of page where CKEditor is placed.

comment:4 Changed 5 years ago by Olek Nowodziński

As for me, inline editors should consider surrounding anchors because they are supposed to integrate as much as possible. Still, div-based editors should act just like framed editor in that case, detecting anchors from editor's content only.

comment:5 Changed 5 years ago by Jakub Ś

I agree div based editor should only be limited to anchors in editor while inline editor should allow accessing anchors in whole page.

comment:6 Changed 5 years ago by Jakub Ś

Status: pendingconfirmed

comment:7 Changed 5 years ago by Piotrek Koszuliński

The question that remains is what should be the default behaviour in each type of editor?

  • framed - only content,
  • inline - entire document or content?
  • divarea - entire document or content? in this case the more general question may be - do we understand divarea as inline editing or is it a lightweight editing in separate context (as framed)?

comment:8 Changed 5 years ago by Jakub Ś

To my understanding:

  • framed - only content,
  • inline - entire document
  • divarea - only content and we understand divarea a lightweight editing in separate context (as framed). If it was to be treated as inline editor what sense would there be to have it? What would the difference be between inline and dive area (except for elements path)?

comment:9 in reply to:  7 ; Changed 5 years ago by Olek Nowodziński

Replying to Reinmar:

The question that remains is what should be the default behaviour in each type of editor?

  • framed - only content,
  • inline - entire document or content?
  • divarea - entire document or content? in this case the more general question may be - do we understand divarea as inline editing or is it a lightweight editing in separate context (as framed)?
  • framed - only content
  • inline - entire document
  • divarea - only content

Divarea is basically framed editing without iframe-ish limitations. Something like Framed++. In fact, this is why we created it and we should follow that path.

comment:10 in reply to:  9 Changed 5 years ago by Frederico Caldeira Knabben

Replying to a.nowodzinski:

  • framed - only content
  • inline - entire document
  • divarea - only content

I agree.

comment:11 Changed 5 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:12 Changed 5 years ago by Olek Nowodziński

Status: assignedreview

t/11359 branch unifies the discovery of anchors according to comment:10.

(+corresponding tests)

comment:13 Changed 5 years ago by Olek Nowodziński

Component: GeneralUI : Dialogs
Milestone: CKEditor 4.3.3
Version: 3.6.6 (SVN - trunk)

comment:14 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  1. No <meta charset> in test file.
  2. Instead of itemsAreSame I would use string comparison (after join(',')). YUI's array assertions do not give clear error messages.
  3. You're not testing empty anchors (which have to be discovered differently, because their representation differs).
  4. I would expose logic retrieving the scope to CKEDITOR.plugins.link. We didn't make it configurable, so let's make it overridable.

comment:15 Changed 5 years ago by Olek Nowodziński

Status: review_failedreview

comment:16 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_passed
* @returns {Array} An array of anchor elements.

May be changed to:

* @returns {CKEDITOR.dom.element[]} An array of anchor elements.

comment:17 Changed 5 years ago by Olek Nowodziński

Resolution: fixed
Status: review_passedclosed

git:05f125f landed in master (c7219cb tests).

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