Problem

Anchor button is enabled in CKeditor, but the name attribute is not allowed in WYSIWYG Filter therefore anchor button / function is not working.

Proposed solution

1) Allow the name attribute in WYSIWYG Filter module / input filter. In that case the href attribute shouldn't be marked as required.
It would be helpful to assess whether allowing the name attribute provides a vector for cross site scripting.

2) Remove Anchor button from WYSIWYG CKEditor.

CommentFileSizeAuthor
#4 commons-add-name-attribute-1479294-4.patch1.23 KBizkreny
#2 commons-add-name-attribute-to-wysiwyg-filter-1479294-2.patch718 bytesizkreny
#1 commons-add-name-attribute-to-wysiwyg-filter-1479294-1.patch719 bytesizkreny

Comments

izkreny’s picture

izkreny commented
Status: Active » Needs review
StatusFileSize
new commons-add-name-attribute-to-wysiwyg-filter-1479294-1.patch719 bytes

And here is the patch - my first one via git - hope so everything is A-OK. :)

izkreny’s picture

izkreny commented
StatusFileSize
new commons-add-name-attribute-to-wysiwyg-filter-1479294-2.patch718 bytes

Bummer, href attribute is required (via exclamation mark), so <a name="anchor"></a> isn't working even name attribute is allowed..

New patch attached.

izkreny’s picture

izkreny commented
Status: Needs review » Needs work

Some pieces are missing. :/
I'll add them soon..

izkreny’s picture

izkreny commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new commons-add-name-attribute-1479294-4.patch1.23 KB

OK, here is the final patch, tested and it's working.

ezra-g’s picture

ezra-g commented
Status: Reviewed & tested by the community » Needs review
Issue tags: -Commons 2.6 radar

Thanks for the patches, @mariomaric!

It's generally not customary to mark your own patches as RTBC :). It would be helpful to assess whether allowing the name attribute provides a vector for cross site scripting.

Marking as "needs review."

izkreny’s picture

izkreny commented

Oops, sorry for RTBC. :)

Hm..OK, if the name attribute is problematic, we should figure some other way for anchors (id?)..or just throw them out from WYSIWYG profile because people get confused - it's there, but it's not working! :/

linkerbox’s picture

linkerbox commented

FYI, the patch did not appear to solve the anchor problem for me. Anchor links still are not working.

izkreny’s picture

izkreny commented
Issue summary: View changes

Updating... -- m.m