Skip to content

Commit

Permalink
Merge branch 'master' into i/6693
Browse files Browse the repository at this point in the history
  • Loading branch information
oleq committed Jun 10, 2020
2 parents d5a86cf + 76c762e commit 893a577
Show file tree
Hide file tree
Showing 54 changed files with 2,501 additions and 1,456 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ describe( 'ArticlePluginSet', () => {
editorElement = document.createElement( 'div' );
document.body.appendChild( editorElement );

editor = await ClassicTestEditor.create( editorElement, { plugins: [ ArticlePluginSet ] } );
editor = await ClassicTestEditor.create( editorElement, {
plugins: [ ArticlePluginSet ],
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
}
} );
} );

afterEach( async () => {
Expand Down
11 changes: 10 additions & 1 deletion packages/ckeditor5-font/tests/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ describe( 'Integration test Font', () => {

return ClassicTestEditor
.create( element, {
plugins: [ Font, ArticlePluginSet ]
plugins: [ Font, ArticlePluginSet ],
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
}
} )
.then( newEditor => {
editor = newEditor;
Expand Down Expand Up @@ -66,6 +69,9 @@ describe( 'Integration test Font', () => {
fontSize: {
options: [ 10, 12, 14 ],
supportAllValues: true
},
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
}
} )
.then( editor => {
Expand Down Expand Up @@ -129,6 +135,9 @@ describe( 'Integration test Font', () => {
fontSize: {
options: [ 10, 12, 14 ],
supportAllValues: true
},
image: {
toolbar: [ 'imageStyle:full', 'imageStyle:side' ]
}
} )
.then( editor => {
Expand Down
6 changes: 1 addition & 5 deletions packages/ckeditor5-image/src/image/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ export function modelToViewAttributeConverter( attributeKey ) {
const figure = conversionApi.mapper.toViewElement( data.item );
const img = getViewImgFromWidget( figure );

if ( data.attributeNewValue !== null ) {
viewWriter.setAttribute( data.attributeKey, data.attributeNewValue, img );
} else {
viewWriter.removeAttribute( data.attributeKey, img );
}
viewWriter.setAttribute( data.attributeKey, data.attributeNewValue || '', img );
}
}
14 changes: 13 additions & 1 deletion packages/ckeditor5-image/src/image/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,23 @@ export function isImageAllowed( model ) {
* Assuming that image is always a first child of a widget (ie. `figureView.getChild( 0 )`) is unsafe as other features might
* inject their own elements to the widget.
*
* The `<img>` can be wrapped to other elements, e.g. `<a>`. Nested check required.
*
* @param {module:engine/view/element~Element} figureView
* @returns {module:engine/view/element~Element}
*/
export function getViewImgFromWidget( figureView ) {
return Array.from( figureView.getChildren() ).find( viewChild => viewChild.is( 'img' ) );
const figureChildren = [];

for ( const figureChild of figureView.getChildren() ) {
figureChildren.push( figureChild );

if ( figureChild.is( 'element' ) ) {
figureChildren.push( ...figureChild.getChildren() );
}
}

return figureChildren.find( viewChild => viewChild.is( 'img' ) );
}

// Checks if image is allowed by schema in optimal insertion parent.
Expand Down
17 changes: 15 additions & 2 deletions packages/ckeditor5-image/tests/image/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,20 @@ describe( 'Image converters', () => {
);
} );

it( 'should convert removing attribute from image', () => {
it( 'should convert an empty "src" attribute from image even if removed', () => {
setModelData( model, '<image src="" alt="foo bar"></image>' );
const image = document.getRoot().getChild( 0 );

model.change( writer => {
writer.removeAttribute( 'src', image );
} );

expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false"><img alt="foo bar" src=""></img></figure>'
);
} );

it( 'should convert an empty "alt" attribute from image even if removed', () => {
setModelData( model, '<image src="" alt="foo bar"></image>' );
const image = document.getRoot().getChild( 0 );

Expand All @@ -227,7 +240,7 @@ describe( 'Image converters', () => {
} );

expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false"><img src=""></img></figure>'
'<figure class="ck-widget image" contenteditable="false"><img alt="" src=""></img></figure>'
);
} );

Expand Down
7 changes: 4 additions & 3 deletions packages/ckeditor5-image/tests/image/imageediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,16 +458,17 @@ describe( 'ImageEditing', () => {
);
} );

it( 'should convert attribute removal', () => {
it( 'should convert attribute removal (but keeps an empty "alt" to the data)', () => {
setModelData( model, '<image src="/assets/sample.png" alt="alt text"></image>' );
const image = doc.getRoot().getChild( 0 );

model.change( writer => {
writer.removeAttribute( 'alt', image );
} );

expect( getViewData( view, { withoutSelection: true } ) )
.to.equal( '<figure class="ck-widget image" contenteditable="false"><img src="/assets/sample.png"></img></figure>' );
expect( getViewData( view, { withoutSelection: true } ) ).to.equal(
'<figure class="ck-widget image" contenteditable="false"><img alt="" src="/assets/sample.png"></img></figure>'
);
} );

it( 'should not convert change if is already consumed', () => {
Expand Down
57 changes: 55 additions & 2 deletions packages/ckeditor5-image/tests/image/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,27 @@ import ViewDocumentFragment from '@ckeditor/ckeditor5-engine/src/view/documentfr
import ViewDowncastWriter from '@ckeditor/ckeditor5-engine/src/view/downcastwriter';
import ViewDocument from '@ckeditor/ckeditor5-engine/src/view/document';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import { toImageWidget, isImageWidget, getSelectedImageWidget, isImage, isImageAllowed, insertImage } from '../../src/image/utils';
import {
toImageWidget,
isImageWidget,
getSelectedImageWidget,
isImage,
isImageAllowed,
insertImage,
getViewImgFromWidget
} from '../../src/image/utils';
import { isWidget, getLabel } from '@ckeditor/ckeditor5-widget/src/utils';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import Image from '../../src/image/imageediting';
import { StylesProcessor } from '@ckeditor/ckeditor5-engine/src/view/stylesmap';

describe( 'image widget utils', () => {
let element, image, writer, viewDocument;

beforeEach( () => {
viewDocument = new ViewDocument();
viewDocument = new ViewDocument( new StylesProcessor() );
writer = new ViewDowncastWriter( viewDocument );
image = writer.createContainerElement( 'img' );
element = writer.createContainerElement( 'figure' );
Expand Down Expand Up @@ -266,4 +275,48 @@ describe( 'image widget utils', () => {
expect( getModelData( model ) ).to.equal( '<other>[]</other>' );
} );
} );

describe( 'getViewImgFromWidget()', () => {
// figure
// img
it( 'returns the the img element from widget if the img is the first children', () => {
expect( getViewImgFromWidget( element ) ).to.equal( image );
} );

// figure
// div
// img
it( 'returns the the img element from widget if the img is not the first children', () => {
writer.insert( writer.createPositionAt( element, 0 ), writer.createContainerElement( 'div' ) );
expect( getViewImgFromWidget( element ) ).to.equal( image );
} );

// figure
// div
// img
it( 'returns the the img element from widget if the img is a child of another element', () => {
const divElement = writer.createContainerElement( 'div' );

writer.insert( writer.createPositionAt( element, 0 ), divElement );
writer.move( writer.createRangeOn( image ), writer.createPositionAt( divElement, 0 ) );

expect( getViewImgFromWidget( element ) ).to.equal( image );
} );

// figure
// div
// "Bar"
// img
// "Foo"
it( 'does not throw an error if text node found', () => {
const divElement = writer.createContainerElement( 'div' );

writer.insert( writer.createPositionAt( element, 0 ), divElement );
writer.insert( writer.createPositionAt( element, 0 ), writer.createText( 'Foo' ) );
writer.insert( writer.createPositionAt( divElement, 0 ), writer.createText( 'Bar' ) );
writer.move( writer.createRangeOn( image ), writer.createPositionAt( divElement, 1 ) );

expect( getViewImgFromWidget( element ) ).to.equal( image );
} );
} );
} );
5 changes: 4 additions & 1 deletion packages/ckeditor5-image/tests/imagetoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* global document */
/* global document, console */

import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
import ImageToolbar from '../src/imagetoolbar';
Expand Down Expand Up @@ -53,6 +53,7 @@ describe( 'ImageToolbar', () => {
} );

it( 'should not initialize if there is no configuration', () => {
const consoleWarnStub = sinon.stub( console, 'warn' );
const editorElement = global.document.createElement( 'div' );
global.document.body.appendChild( editorElement );

Expand All @@ -61,6 +62,8 @@ describe( 'ImageToolbar', () => {
} )
.then( editor => {
expect( editor.plugins.get( ImageToolbar )._toolbar ).to.be.undefined;
expect( consoleWarnStub.calledOnce ).to.equal( true );
expect( consoleWarnStub.firstCall.args[ 0 ] ).to.match( /^widget-toolbar-no-items:/ );

editorElement.remove();
return editor.destroy();
Expand Down
6 changes: 5 additions & 1 deletion packages/ckeditor5-image/tests/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Image from '../src/image';
import ImageToolbar from '../src/imagetoolbar';
import View from '@ckeditor/ckeditor5-ui/src/view';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import ImageTextAlternative from '../src/imagetextalternative';

describe( 'ImageToolbar integration', () => {
describe( 'with the BalloonToolbar', () => {
Expand All @@ -25,7 +26,10 @@ describe( 'ImageToolbar integration', () => {

return ClassicTestEditor
.create( editorElement, {
plugins: [ Image, ImageToolbar, BalloonToolbar, Paragraph ]
plugins: [ Image, ImageTextAlternative, ImageToolbar, BalloonToolbar, Paragraph ],
image: {
toolbar: [ 'imageTextAlternative' ]
}
} )
.then( editor => {
newEditor = editor;
Expand Down
24 changes: 24 additions & 0 deletions packages/ckeditor5-link/docs/features/link.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,30 @@ ClassicEditor
.catch( ... );
```

#### Adding default link protocol for the external links

Default link protocol can be usefull when user forget to type a full URL address to an external source, site etc. Sometimes copying the text, like for example `ckeditor.com` and converting it to a link may cause some issues. When you do this, the created link will direct you to `yourdomain.com/ckeditor.com`, because you forgot to pass the right protocol which makes the link relative to the site where it appears.

Enabling the `{@link module:link/link~LinkConfig#defaultProtocol config.link.defaultProtocol}`, the {@link module:link/link~Link} feature will handle this issue for you. By default it doesn't fix the passed link value, but when you set `{@link module:link/link~LinkConfig#defaultProtocol config.link.defaultProtocol}` to — for example — `http://`, the plugin will add the given protocol to the every link that may need it (like `ckeditor.com`, `example.com` etc. where `[protocol://]example.com` is missing). Here's the basic configuration example:

```js
ClassicEditor
.create( document.querySelector( '#editor' ), {
// ...
link: {
defaultProtocol: 'http://'
}
} )
.then( ... )
.catch( ... );
```

<info-box>
Having `config.link.defaultProtocol` enabled you are still able to link things locally using `#` or `/`. Protocol won't be added to those links.

Enabled feature also gives you an **email addresses auto-detection** feature. When you submit `[email protected]`, the plugin will change it automatically to `mailto:[email protected]`.
</info-box>

#### Adding attributes to links based on pre–defined rules (automatic decorators)

Automatic link decorators match all links in the editor content against a {@link module:link/link~LinkDecoratorAutomaticDefinition function} which decides whether the link should receive some set of attributes, considering the URL (`href`) of the link. These decorators work silently and are being applied during the {@link framework/guides/architecture/editing-engine#conversion data downcast} only.
Expand Down
2 changes: 2 additions & 0 deletions packages/ckeditor5-link/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
"lodash-es": "^4.17.15"
},
"devDependencies": {
"@ckeditor/ckeditor5-basic-styles": "^19.0.1",
"@ckeditor/ckeditor5-block-quote": "^19.0.1",
"@ckeditor/ckeditor5-clipboard": "^19.0.1",
"@ckeditor/ckeditor5-editor-classic": "^19.0.1",
"@ckeditor/ckeditor5-enter": "^19.0.1",
"@ckeditor/ckeditor5-image": "^19.0.1",
"@ckeditor/ckeditor5-paragraph": "^19.1.0",
"@ckeditor/ckeditor5-theme-lark": "^19.1.0",
"@ckeditor/ckeditor5-typing": "^19.0.1",
Expand Down
22 changes: 22 additions & 0 deletions packages/ckeditor5-link/src/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,28 @@ export default class Link extends Plugin {
* @interface LinkConfig
*/

/**
* When set, the editor will add the given protocol to the link when the user creates a link without one.
* For example, when the user is creating a link and types `ckeditor.com` in the link form input — during link submission —
* the editor will automatically add the `http://` protocol, so the link will be as follows: `http://ckeditor.com`.
*
* The feature also comes with an email auto-detection. When you submit `[email protected]`
* the plugin will automatically change it to `mailto:[email protected]`.
*
* ClassicEditor
* .create( editorElement, {
* link: {
* defaultProtocol: 'http://'
* }
* } )
* .then( ... )
* .catch( ... );
*
* **NOTE:** In case no configuration is provided, the editor won't auto-fix the links.
*
* @member {String} module:link/link~LinkConfig#defaultProtocol
*/

/**
* When set to `true`, the `target="blank"` and `rel="noopener noreferrer"` attributes are automatically added to all external links
* in the editor. "External links" are all links in the editor content starting with `http`, `https`, or `//`.
Expand Down
41 changes: 40 additions & 1 deletion packages/ckeditor5-link/src/linkcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,30 @@ export default class LinkCommand extends Command {
}
} else {
// If selection has non-collapsed ranges, we change attribute on nodes inside those ranges
// omitting nodes where `linkHref` attribute is disallowed.
// omitting nodes where the `linkHref` attribute is disallowed.
const ranges = model.schema.getValidRanges( selection.getRanges(), 'linkHref' );

// But for the first, check whether the `linkHref` attribute is allowed on selected blocks (e.g. the "image" element).
const allowedRanges = [];

for ( const element of selection.getSelectedBlocks() ) {
if ( model.schema.checkAttribute( element, 'linkHref' ) ) {
allowedRanges.push( writer.createRangeOn( element ) );
}
}

// Ranges that accept the `linkHref` attribute. Since we will iterate over `allowedRanges`, let's clone it.
const rangesToUpdate = allowedRanges.slice();

// For all selection ranges we want to check whether given range is inside an element that accepts the `linkHref` attribute.
// If so, we don't want to propagate applying the attribute to its children.
for ( const range of ranges ) {
if ( this._isRangeToUpdate( range, allowedRanges ) ) {
rangesToUpdate.push( range );
}
}

for ( const range of rangesToUpdate ) {
writer.setAttribute( 'linkHref', href, range );

truthyManualDecorators.forEach( item => {
Expand All @@ -216,4 +236,23 @@ export default class LinkCommand extends Command {
const doc = this.editor.model.document;
return doc.selection.getAttribute( decoratorName );
}

/**
* Checks whether specified `range` is inside an element that accepts the `linkHref` attribute.
*
* @private
* @param {module:engine/view/range~Range} range A range to check.
* @param {Array.<module:engine/view/range~Range>} allowedRanges An array of ranges created on elements where the attribute is accepted.
* @returns {Boolean}
*/
_isRangeToUpdate( range, allowedRanges ) {
for ( const allowedRange of allowedRanges ) {
// A range is inside an element that will have the `linkHref` attribute. Do not modify its nodes.
if ( allowedRange.containsRange( range ) ) {
return false;
}
}

return true;
}
}
Loading

0 comments on commit 893a577

Please sign in to comment.