Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Content Model Bug bash fix part 3: More content fidelity fixes #1378

Merged
merged 15 commits into from
Nov 4, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ContentModelBlockFormat, ContentModelSegmentFormat } from 'roosterjs-co
import { DirectionFormatRenderers } from './formatPart/DirectionFormatRenderers';
import { FormatRenderer } from './utils/FormatRenderer';
import { FormatView } from './FormatView';
import { LineHeightFormatRenderer } from './formatPart/LineHeightFormatRenderer';
import { MarginFormatRenderer } from './formatPart/MarginFormatRenderer';
import { PaddingFormatRenderer } from './formatPart/PaddingFormatRenderer';

Expand All @@ -12,6 +13,7 @@ const BlockFormatRenders: FormatRenderer<ContentModelBlockFormat>[] = [
...DirectionFormatRenderers,
MarginFormatRenderer,
PaddingFormatRenderer,
LineHeightFormatRenderer,
];

export function BlockFormatView(props: { format: ContentModelSegmentFormat }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import { FormatView } from './FormatView';
import { LinkFormat } from 'roosterjs-content-model';

const LinkFormatRenderers: FormatRenderer<LinkFormat>[] = [
createTextFormatRenderer<LinkFormat>(
'Name',
format => format.name,
(format, value) => (format.name = value)
),
createTextFormatRenderer<LinkFormat>(
'Href',
format => format.href,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ import {
UnderlineFormat,
} from 'roosterjs-content-model';

export const BoldFormatRenderer: FormatRenderer<BoldFormat> = createCheckboxFormatRenderer<
BoldFormat
>(
export const BoldFormatRenderer: FormatRenderer<BoldFormat> = createTextFormatRenderer<BoldFormat>(
'Bold',
format => format.bold,
(format, value) => (format.bold = value)
format => format.fontWeight,
(format, value) => (format.fontWeight = value)
);

export const ItalicFormatRenderer: FormatRenderer<ItalicFormat> = createCheckboxFormatRenderer<
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createTextFormatRenderer } from '../utils/createTextFormatRenderer';
import { DisplayFormat } from 'roosterjs-content-model';
import { FormatRenderer } from '../utils/FormatRenderer';

export const DisplayFormatRenderer: FormatRenderer<DisplayFormat> = createTextFormatRenderer<
DisplayFormat
>(
'Display',
format => format.display,
(format, value) => (format.display = value)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createTextFormatRenderer } from '../utils/createTextFormatRenderer';
import { FormatRenderer } from '../utils/FormatRenderer';
import { LineHeightFormat } from 'roosterjs-content-model';

export const LineHeightFormatRenderer: FormatRenderer<LineHeightFormat> = createTextFormatRenderer<
LineHeightFormat
>(
'LineHeight',
format => format.lineHeight,
(format, value) => (format.lineHeight = value)
);
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { BorderBoxFormatRenderer } from '../format/formatPart/BorderBoxFormatRen
import { BorderFormatRenderers } from '../format/formatPart/BorderFormatRenderers';
import { ContentModelBlockGroupView } from './ContentModelBlockGroupView';
import { ContentModelView } from '../ContentModelView';
import { DisplayFormatRenderer } from '../format/formatPart/DisplayFormatRenderer';
import { FormatRenderer } from '../format/utils/FormatRenderer';
import { FormatView } from '../format/FormatView';
import { IdFormatRenderer } from '../format/formatPart/IdFormatRenderer';
Expand All @@ -28,6 +29,7 @@ const TableFormatRenderers: FormatRenderer<ContentModelTableFormat>[] = [
...BorderFormatRenderers,
BorderBoxFormatRenderer,
...TableMetadataFormatRenders,
DisplayFormatRenderer,
];

export function ContentModelTableView(props: { table: ContentModelTable }) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { brProcessor } from '../processors/brProcessor';
import { childProcessor } from '../processors/childProcessor';
import { createTempContainerProcessor } from '../processors/tempContainerProcessor';
import { elementProcessor } from '../processors/elementProcessor';
import { ElementProcessorMap } from '../../publicTypes/context/DomToModelSettings';
import { entityProcessor } from '../processors/entityProcessor';
Expand All @@ -15,8 +14,6 @@ import { quoteProcessor } from '../processors/quoteProcessor';
import { tableProcessor } from '../processors/tableProcessor';
import { textProcessor } from '../processors/textProcessor';

const tempContainerProcessor = createTempContainerProcessor();

/**
* @internal
*/
Expand All @@ -25,22 +22,23 @@ export const defaultProcessorMap: ElementProcessorMap = {
b: knownElementProcessor,
blockquote: quoteProcessor,
br: brProcessor,
div: tempContainerProcessor,
div: knownElementProcessor,
em: knownElementProcessor,
font: fontProcessor,
i: knownElementProcessor,
img: imageProcessor,
h1: tempContainerProcessor,
h2: tempContainerProcessor,
h3: tempContainerProcessor,
h4: tempContainerProcessor,
h5: tempContainerProcessor,
h6: tempContainerProcessor,
h1: knownElementProcessor,
h2: knownElementProcessor,
h3: knownElementProcessor,
h4: knownElementProcessor,
h5: knownElementProcessor,
h6: knownElementProcessor,
hr: hrProcessor,
li: listItemProcessor,
ol: listProcessor,
p: knownElementProcessor,
s: knownElementProcessor,
span: tempContainerProcessor,
span: knownElementProcessor,
strike: knownElementProcessor,
strong: knownElementProcessor,
sub: knownElementProcessor,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const knownElementProcessor: ElementProcessor<HTMLElement> = (group, elem
);

if (isBlock) {
addBlock(group, createParagraph(false /*isImplicit*/, context.blockFormat));
addBlock(group, createParagraph(true /*isImplicit*/, context.blockFormat));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { createText } from '../../modelApi/creators/createText';
import { DomToModelContext } from '../../publicTypes/context/DomToModelContext';
import { ElementProcessor } from '../../publicTypes/context/ElementProcessor';
import { getRegularSelectionOffsets } from '../utils/getRegularSelectionOffsets';
import { hasSpacesOnly } from '../../domUtils/hasSpacesOnly';

/**
* @internal
Expand Down Expand Up @@ -44,20 +45,18 @@ export const textProcessor: ElementProcessor<Text> = (

function addTextSegment(group: ContentModelBlockGroup, text: string, context: DomToModelContext) {
if (text) {
const paragraph = group.blocks[group.blocks.length - 1];
const lastSegment =
paragraph?.blockType == 'Paragraph' &&
paragraph.segments[paragraph.segments.length - 1];
const lastBlock = group.blocks[group.blocks.length - 1];
const paragraph = lastBlock?.blockType == 'Paragraph' ? lastBlock : null;
const lastSegment = paragraph?.segments[paragraph.segments.length - 1];

if (
lastSegment &&
lastSegment.segmentType == 'Text' &&
lastSegment?.segmentType == 'Text' &&
!!lastSegment.isSelected == !!context.isInSelection &&
areSameFormats(lastSegment.format, context.segmentFormat) &&
areSameFormats(lastSegment.link || {}, context.linkFormat.format || {})
) {
lastSegment.text += text;
} else {
} else if (!hasSpacesOnly(text) || paragraph?.segments.length! > 0) {
const textModel = createText(text, context.segmentFormat, context.linkFormat.format);

if (context.isInSelection) {
Expand Down
10 changes: 10 additions & 0 deletions packages/roosterjs-content-model/lib/domUtils/hasSpacesOnly.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// A regex to match text that only has space and CR
// We use real space char " " (\u0020) here but not "\s" since "\s" will also match "&nbsp;" (\u00A0) which is something we need to keep
const SPACE_TEXT_REGEX = /^[\r\n\t ]*$/;

/**
* @internal
*/
export function hasSpacesOnly(txt: string): boolean {
return SPACE_TEXT_REGEX.test(txt);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { DisplayFormat } from '../../publicTypes/format/formatParts/DisplayFormat';
import { FormatHandler } from '../FormatHandler';

/**
* @internal
*/
export const displayFormatHandler: FormatHandler<DisplayFormat> = {
parse: (format, element) => {
const display = element.style.display;

if (display) {
format.display = display;
}
},
apply: (format, element) => {
if (format.display) {
element.style.display = format.display;
}
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { FormatHandler } from '../FormatHandler';
import { LineHeightFormat } from '../../publicTypes/format/formatParts/LineHeightFormat';

/**
* @internal
*/
export const lineHeightFormatHandler: FormatHandler<LineHeightFormat> = {
parse: (format, element, context, defaultStyle) => {
const lineHeight = element.style.lineHeight || defaultStyle.lineHeight;

if (lineHeight && lineHeight != 'inherit') {
format.lineHeight = lineHeight;
}
},
apply: (format, element) => {
if (format.lineHeight) {
element.style.lineHeight = format.lineHeight;
}
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { borderBoxFormatHandler } from './common/borderBoxFormatHandler';
import { borderFormatHandler } from './common/borderFormatHandler';
import { ContentModelFormatMap } from '../publicTypes/format/ContentModelFormatMap';
import { directionFormatHandler } from './block/directionFormatHandler';
import { displayFormatHandler } from './block/displayFormatHandler';
import { fontFamilyFormatHandler } from './segment/fontFamilyFormatHandler';
import { fontSizeFormatHandler } from './segment/fontSizeFormatHandler';
import { FormatHandler } from './FormatHandler';
Expand All @@ -12,6 +13,7 @@ import { getObjectKeys } from 'roosterjs-editor-dom';
import { idFormatHandler } from './common/idFormatHandler';
import { imageMetadataFormatHandler } from './image/imageMetadataFormatHandler';
import { italicFormatHandler } from './segment/italicFormatHandler';
import { lineHeightFormatHandler } from './block/lineHeightFormatHandler';
import { linkFormatHandler } from './segment/linkFormatHandler';
import { listItemMetadataFormatHandler } from './list/listItemMetadataFormatHandler';
import { listItemThreadFormatHandler } from './list/listItemThreadFormatHandler';
Expand Down Expand Up @@ -50,11 +52,13 @@ const defaultFormatHandlerMap: FormatHandlers = {
border: borderFormatHandler,
borderBox: borderBoxFormatHandler,
direction: directionFormatHandler,
display: displayFormatHandler,
fontFamily: fontFamilyFormatHandler,
fontSize: fontSizeFormatHandler,
id: idFormatHandler,
imageMetadata: imageMetadataFormatHandler,
italic: italicFormatHandler,
lineHeight: lineHeightFormatHandler,
link: linkFormatHandler,
listItemMetadata: listItemMetadataFormatHandler,
listItemThread: listItemThreadFormatHandler,
Expand All @@ -77,7 +81,7 @@ const defaultFormatHandlerMap: FormatHandlers = {
const defaultFormatKeysPerCategory: {
[key in keyof ContentModelFormatMap]: (keyof FormatHandlerTypeMap)[];
} = {
block: ['backgroundColor', 'direction', 'margin', 'padding'],
block: ['backgroundColor', 'direction', 'margin', 'padding', 'lineHeight'],
listItem: ['listItemThread', 'listItemMetadata'],
listLevel: ['listType', 'listLevelThread', 'listLevelMetadata'],
segment: [
Expand Down Expand Up @@ -109,6 +113,8 @@ const defaultFormatKeysPerCategory: {
'tableSpacing',
'margin',
'backgroundColor',
'display',
'direction',
],
image: ['id', 'size', 'margin', 'padding', 'borderBox', 'imageMetadata'],
link: ['link'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,26 @@ import { moveChildNodes } from 'roosterjs-editor-dom';
*/
export const boldFormatHandler: FormatHandler<BoldFormat> = {
parse: (format, element, context, defaultStyle) => {
const fontWeight = element.style.fontWeight || defaultStyle.fontWeight || '';
const fontWeightInt = parseInt(fontWeight);
const fontWeight = element.style.fontWeight || defaultStyle.fontWeight;

if (fontWeight == 'bold' || fontWeight == 'bolder' || fontWeightInt >= 600) {
format.bold = true;
} else if (
fontWeight == 'normal' ||
fontWeight == 'lighter' ||
fontWeight == 'initial' ||
fontWeightInt < 600
) {
format.bold = false;
if (fontWeight) {
format.fontWeight = fontWeight;
}
},
apply: (format, element, context) => {
if (format.bold && !context.segmentFormatFromBlock.bold) {
const b = element.ownerDocument.createElement('b');
moveChildNodes(b, element);
element.appendChild(b);
} else if (!format.bold && context.segmentFormatFromBlock.bold) {
element.style.fontWeight = 'normal';
const blockFontWeight = context.segmentFormatFromBlock.fontWeight;

if (
(blockFontWeight && blockFontWeight != format.fontWeight) ||
(!blockFontWeight && format.fontWeight && format.fontWeight != 'normal')
) {
if (format.fontWeight == 'bold') {
const b = element.ownerDocument.createElement('b');
moveChildNodes(b, element);
element.appendChild(b);
} else {
element.style.fontWeight = format.fontWeight || 'normal';
}
}
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const fontFamilyFormatHandler: FormatHandler<FontFamilyFormat> = {
parse: (format, element, context, defaultStyle) => {
const fontFamily = element.style.fontFamily || defaultStyle.fontFamily;

if (fontFamily) {
if (fontFamily && fontFamily != 'inherit') {
format.fontFamily = fontFamily;
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const fontSizeFormatHandler: FormatHandler<FontSizeFormat> = {

// when font size is 'smaller' and the style is for superscript/subscript,
// the font size will be handled by superOrSubScript handler
if (fontSize && !isSuperOrSubScript(fontSize, verticalAlign)) {
if (fontSize && !isSuperOrSubScript(fontSize, verticalAlign) && fontSize != 'inherit') {
format.fontSize = fontSize;
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@ import { safeInstanceOf } from 'roosterjs-editor-dom';
export const linkFormatHandler: FormatHandler<LinkFormat> = {
parse: (format, element) => {
if (safeInstanceOf(element, 'HTMLAnchorElement')) {
const name = element.name;
const href = element.getAttribute('href'); // Use getAttribute to get original HREF but not the resolved absolute url
const target = element.target;
const rel = element.rel;
const id = element.id;
const className = element.className;
const title = element.title;

if (name) {
format.name = name;
}

if (href) {
format.href = href;
}
Expand Down Expand Up @@ -44,6 +49,10 @@ export const linkFormatHandler: FormatHandler<LinkFormat> = {
if (safeInstanceOf(element, 'HTMLAnchorElement') && format.href) {
element.href = format.href;

if (format.name) {
element.name = format.name;
}

if (format.target) {
element.target = format.target;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const textColorFormatHandler: FormatHandler<TextColorFormat> = {
const textColor =
getColor(element, false /*isBackground*/, context.isDarkMode) || defaultStyle.color;

if (textColor) {
if (textColor && textColor != 'inherit') {
format.textColor = textColor;
}
},
Expand Down
2 changes: 2 additions & 0 deletions packages/roosterjs-content-model/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export { TableMetadataFormat } from './publicTypes/format/formatParts/TableMetad
export { ContentModelFormatBase } from './publicTypes/format/ContentModelFormatBase';
export { MarginFormat } from './publicTypes/format/formatParts/MarginFormat';
export { PaddingFormat } from './publicTypes/format/formatParts/PaddingFormat';
export { DisplayFormat } from './publicTypes/format/formatParts/DisplayFormat';
export { LineHeightFormat } from './publicTypes/format/formatParts/LineHeightFormat';
export { LinkFormat } from './publicTypes/format/formatParts/LinkFormat';
export { ListTypeFormat } from './publicTypes/format/formatParts/ListTypeFormat';
export { ListThreadFormat } from './publicTypes/format/formatParts/ListThreadFormat';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export function isBlockGroupEmpty(group: ContentModelBlockGroup): boolean {
export function isSegmentEmpty(segment: ContentModelSegment): boolean {
switch (segment.segmentType) {
case 'Text':
return !segment.text || /^[\r\n\s\t]*$/.test(segment.text);
return !segment.text;

case 'Image':
return !segment.src;
Expand Down
Loading