Skip to content

Commit 71e7ca7

Browse files
mlewandmlewand
committed
Merge branch 't/14538'
2 parents e416daf + 3a30e8e commit 71e7ca7

File tree

5 files changed

+167
-74
lines changed

5 files changed

+167
-74
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CKEditor 4 Changelog
55

66
Fixed Issues:
77

8+
* [#14538](http://dev.ckeditor.com/ticket/14538): Fixed: Keyboard focus goes into embedded iframe element.
89
* [#14602](http://dev.ckeditor.com/ticket/14602): Fixed: [dom.element.removeAttribute()](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.element-method-removeAttribute) method does not remove all attributes if no parameter is given.
910
* [#8679](http://dev.ckeditor.com/ticket/8679): Fixed: Better focus indication and ability to style selected color in the color picker dialog.
1011
* [#14312](http://dev.ckeditor.com/ticket/14312): Fixed: [IE] Artifact is visible after pasting of any text.

‎plugins/embedbase/plugin.js

Lines changed: 78 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,84 @@
1-
/**
1+
/**
22
* @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
33
* For licensing, see LICENSE.md or http://ckeditor.com/license
44
*/
55

66
( function() {
77
'use strict';
88

9+
/**
10+
* JSONP communication.
11+
*
12+
* @private
13+
* @singleton
14+
* @class CKEDITOR.plugins.embedBase._jsonp
15+
*/
16+
var Jsonp = {
17+
/**
18+
* Creates a `<script>` element and attaches it to the document `<body>`.
19+
*
20+
* @private
21+
*/
22+
_attachScript: function( url, errorCallback ) {
23+
// ATM we cannot use CKE scriptloader here, because it will make sure that script
24+
// with given URL is added only once.
25+
var script = new CKEDITOR.dom.element( 'script' );
26+
script.setAttribute( 'src', url );
27+
script.on( 'error', errorCallback );
28+
29+
CKEDITOR.document.getBody().append( script );
30+
31+
return script;
32+
},
33+
34+
/**
35+
* Sends a request using the JSONP technique.
36+
*
37+
* @param {CKEDITOR.template} urlTemplate The template of the URL to be requested. All properties
38+
* passed in `urlParams` can be used, plus a `{callback}`, which represent a JSONP callback, must be defined.
39+
* @param {Object} urlParams Parameters to be passed to the `urlTemplate`.
40+
* @param {Function} callback
41+
* @param {Function} [errorCallback]
42+
* @returns {Object} The request object with a `cancel()` method.
43+
*/
44+
sendRequest: function( urlTemplate, urlParams, callback, errorCallback ) {
45+
var request = {};
46+
urlParams = urlParams || {};
47+
48+
var callbackKey = CKEDITOR.tools.getNextNumber(),
49+
scriptElement;
50+
51+
urlParams.callback = 'CKEDITOR._.jsonpCallbacks[' + callbackKey + ']';
52+
53+
CKEDITOR._.jsonpCallbacks[ callbackKey ] = function( response ) {
54+
// On IEs scripts are sometimes loaded synchronously. It is bad for two reasons:
55+
// * nature of sendRequest() is unstable,
56+
// * scriptElement does not exist yet.
57+
setTimeout( function() {
58+
cleanUp();
59+
callback( response );
60+
} );
61+
};
62+
63+
scriptElement = this._attachScript( urlTemplate.output( urlParams ), function() {
64+
cleanUp();
65+
errorCallback && errorCallback();
66+
} );
67+
68+
request.cancel = cleanUp;
69+
70+
function cleanUp() {
71+
if ( scriptElement ) {
72+
scriptElement.remove();
73+
delete CKEDITOR._.jsonpCallbacks[ callbackKey ];
74+
scriptElement = null;
75+
}
76+
}
77+
78+
return request;
79+
}
80+
};
81+
982
CKEDITOR.plugins.add( 'embedbase', {
1083
lang: 'cs,da,de,de-ch,en,eo,eu,fr,gl,id,it,ko,ku,nb,nl,pl,pt-br,ru,sv,tr,ug,uk,zh,zh-cn', // %REMOVE_LINE_CORE%
1184
requires: 'widget,notificationaggregator',
@@ -394,6 +467,10 @@
394467
return '<img src="' + CKEDITOR.tools.htmlEncodeAttr( response.url ) + '" ' +
395468
'alt="' + CKEDITOR.tools.htmlEncodeAttr( response.title || '' ) + '" style="max-width:100%;height:auto" />';
396469
} else if ( response.type == 'video' || response.type == 'rich' ) {
470+
// Embedded iframes are added to page's focus list. Adding negative tabindex attribute
471+
// removes their ability to be focused by user. (#14538)
472+
response.html = response.html.replace( /<iframe/g, '<iframe tabindex="-1"' );
473+
397474
return response.html;
398475
}
399476

@@ -521,79 +598,6 @@
521598
*/
522599
}
523600

524-
/**
525-
* JSONP communication.
526-
*
527-
* @private
528-
* @singleton
529-
* @class CKEDITOR.plugins.embedBase._jsonp
530-
*/
531-
var Jsonp = {
532-
/**
533-
* Creates a `<script>` element and attaches it to the document `<body>`.
534-
*
535-
* @private
536-
*/
537-
_attachScript: function( url, errorCallback ) {
538-
// ATM we cannot use CKE scriptloader here, because it will make sure that script
539-
// with given URL is added only once.
540-
var script = new CKEDITOR.dom.element( 'script' );
541-
script.setAttribute( 'src', url );
542-
script.on( 'error', errorCallback );
543-
544-
CKEDITOR.document.getBody().append( script );
545-
546-
return script;
547-
},
548-
549-
/**
550-
* Sends a request using the JSONP technique.
551-
*
552-
* @param {CKEDITOR.template} urlTemplate The template of the URL to be requested. All properties
553-
* passed in `urlParams` can be used, plus a `{callback}`, which represent a JSONP callback, must be defined.
554-
* @param {Object} urlParams Parameters to be passed to the `urlTemplate`.
555-
* @param {Function} callback
556-
* @param {Function} [errorCallback]
557-
* @returns {Object} The request object with a `cancel()` method.
558-
*/
559-
sendRequest: function( urlTemplate, urlParams, callback, errorCallback ) {
560-
var request = {};
561-
urlParams = urlParams || {};
562-
563-
var callbackKey = CKEDITOR.tools.getNextNumber(),
564-
scriptElement;
565-
566-
urlParams.callback = 'CKEDITOR._.jsonpCallbacks[' + callbackKey + ']';
567-
568-
CKEDITOR._.jsonpCallbacks[ callbackKey ] = function( response ) {
569-
// On IEs scripts are sometimes loaded synchronously. It is bad for two reasons:
570-
// * nature of sendRequest() is unstable,
571-
// * scriptElement does not exist yet.
572-
setTimeout( function() {
573-
cleanUp();
574-
callback( response );
575-
} );
576-
};
577-
578-
scriptElement = this._attachScript( urlTemplate.output( urlParams ), function() {
579-
cleanUp();
580-
errorCallback && errorCallback();
581-
} );
582-
583-
request.cancel = cleanUp;
584-
585-
function cleanUp() {
586-
if ( scriptElement ) {
587-
scriptElement.remove();
588-
delete CKEDITOR._.jsonpCallbacks[ callbackKey ];
589-
scriptElement = null;
590-
}
591-
}
592-
593-
return request;
594-
}
595-
};
596-
597601
/**
598602
* Class representing the request object. It is created by the {@link CKEDITOR.plugins.embedBase.baseDefinition#loadContent}
599603
* method and is passed to other methods and events of this class.

‎tests/plugins/embed/iframefocus.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/* bender-tags: editor, embed, 14538 */
2+
/* bender-ckeditor-plugins: embed */
3+
/* bender-include: ../embedbase/_helpers/tools.js */
4+
/* global embedTools */
5+
6+
bender.editor = {
7+
config: {
8+
allowedContent: true
9+
}
10+
};
11+
12+
embedTools.mockJsonp( function( template, data, callback ) {
13+
callback( {
14+
type: 'rich',
15+
html: '<iframe src="http://video"></iframe>'
16+
} );
17+
} );
18+
19+
bender.test( {
20+
'test support for removing Tab stop on iframes': function() {
21+
var bot = this.editorBot,
22+
editor = bot.editor;
23+
24+
editor.widgets.once( 'instanceCreated', function( evt ) {
25+
evt.data.once( 'handleResponse', function() {
26+
resume( function() {
27+
assert.areSame( '-1', editor.editable().findOne( 'iframe' ).getAttribute( 'tabindex' ) );
28+
} );
29+
} );
30+
} );
31+
32+
bot.setData( '', function() {
33+
bot.dialog( 'embed', function( dialog ) {
34+
dialog.setValueOf( 'info', 'url', 'http://video' );
35+
dialog.getButton( 'ok' ).click();
36+
37+
wait();
38+
} );
39+
} );
40+
}
41+
} );

‎tests/plugins/embed/manual/iframefocus.html

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<head>
2+
<style>
3+
button:focus {
4+
border: 3px #f00 solid;
5+
}
6+
</style>
7+
</head>
8+
<p>Test URLs</p>
9+
10+
<ul>
11+
<li>https://vimeo.com/86541796</li>
12+
<li>https://www.youtube.com/watch?v=cakkDjbBKmg</li>
13+
</ul>
14+
15+
<p>Dat rich editor</p>
16+
17+
<textarea id="editor1" cols="10" rows="10">
18+
<p>Foo bar</p>
19+
<p>Foo bar.</p>
20+
</textarea>
21+
22+
<p><button>Focus indicator</button></p>
23+
24+
<script>
25+
CKEDITOR.replace( 'editor1' );
26+
</script>

‎tests/plugins/embed/manual/iframefocus.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
@bender-tags: embed, 14538, tc, 4.5.9
2+
@bender-ui: collapsed
3+
@bender-ckeditor-plugins: embed,wysiwygarea,sourcearea,htmlwriter,entities,toolbar,elementspath,undo,clipboard
4+
5+
**Procedure:**
6+
7+
1. Embed URL from "Test URLs" list between paragraphs inside the editor.
8+
2. Press right arrow to move focus from widget to the following paragraph.
9+
3. Press <kbd>Tab</kbd>.
10+
11+
**Expected:**
12+
13+
Focus is moved to the button following the editor.
14+
15+
**Unexpected:**
16+
17+
Focus is moved inside embedded video's `iframe`.
18+
19+
Notes:
20+
21+
* Many iframely's iframes throw errors on IEs (8-11) - this isn't our fault. Warnings on other browsers aren't our fault too...

0 commit comments

Comments
 (0)