jenkins-bot has submitted this change and it was merged.
Change subject: Restore original rowspan/colspan rather than normalizing or
overwriting with NaN
......................................................................
Restore original rowspan/colspan rather than normalizing or overwriting with NaN
Store originalRowspan/Colspan as strings instead of numbers,
so we get the actual original value. Use the original value
if the number we're setting is not really different
(prevents normalization of "02" to "2") or is NaN
(prevents normalization of "2 blah" to "NaN").
Bonus: don't modify dataElement.attributes
Change-Id: Ic30e0d6f7bfe27c10e505b7c18fe9200f322cfc7
---
M src/dm/nodes/ve.dm.TableCellNode.js
M tests/dm/ve.dm.example.js
2 files changed, 46 insertions(+), 12 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/dm/nodes/ve.dm.TableCellNode.js
b/src/dm/nodes/ve.dm.TableCellNode.js
index ba267d1..85e2dd4 100644
--- a/src/dm/nodes/ve.dm.TableCellNode.js
+++ b/src/dm/nodes/ve.dm.TableCellNode.js
@@ -52,12 +52,12 @@
if ( colspan !== null && colspan !== '' ) {
attributes.colspan = Number( colspan );
- attributes.originalColspan = attributes.colspan;
+ attributes.originalColspan = colspan;
}
if ( rowspan !== null && rowspan !== '' ) {
attributes.rowspan = Number( rowspan );
- attributes.originalRowspan = attributes.rowspan;
+ attributes.originalRowspan = rowspan;
}
return {
@@ -69,18 +69,31 @@
ve.dm.TableCellNode.static.toDomElements = function ( dataElement, doc ) {
var tag = dataElement.attributes && dataElement.attributes.style ===
'header' ? 'th' : 'td',
domElement = doc.createElement( tag ),
- attributes = dataElement.attributes;
+ attributes = dataElement.attributes,
+ spans = {
+ colspan: attributes.colspan,
+ rowspan: attributes.rowspan
+ };
// Ignore spans of 1 unless they were in the original HTML
- if ( attributes.colspan === 1 && !attributes.originalColspan ) {
- attributes.colspan = null;
+ if ( attributes.colspan === 1 && Number( attributes.originalColspan )
!== 1 ) {
+ spans.colspan = null;
}
- if ( attributes.rowspan === 1 && !attributes.originalRowspan ) {
- attributes.rowspan = null;
+ if ( attributes.rowspan === 1 && Number( attributes.originalRowspan )
!== 1 ) {
+ spans.rowspan = null;
}
- ve.setDomAttributes( domElement, dataElement.attributes, [ 'colspan',
'rowspan' ] );
+ // Use original value if the numerical value didn't change, or if the
numerical value is NaN
+ if ( attributes.colspan === Number( attributes.originalColspan ) ||
isNaN( attributes.colspan ) ) {
+ spans.colspan = attributes.originalColspan;
+ }
+
+ if ( attributes.rowspan === Number( attributes.originalRowspan ) ||
isNaN( attributes.rowspan ) ) {
+ spans.rowspan = attributes.originalRowspan;
+ }
+
+ ve.setDomAttributes( domElement, spans );
return [ domElement ];
};
diff --git a/tests/dm/ve.dm.example.js b/tests/dm/ve.dm.example.js
index 515bb85..7f3c4e9 100644
--- a/tests/dm/ve.dm.example.js
+++ b/tests/dm/ve.dm.example.js
@@ -776,7 +776,7 @@
];
ve.dm.example.complexTableHtml =
'<table><caption>Foo</caption><thead><tr><th>Bar</th></tr></thead>' +
-
'<tfoot><tr><td>Baz</td></tr></tfoot><tbody><tr><td>Quux</td><td>Whee</td></tr></tbody></table>';
+ '<tfoot><tr><td colspan=2>Baz</td></tr></tfoot><tbody><tr><td
rowspan="02">Quux</td><td colspan="2 garbage">Whee</td></tr></tbody></table>';
ve.dm.example.complexTable = [
{ type: 'table' },
@@ -800,7 +800,14 @@
{ type: '/tableSection' },
{ type: 'tableSection', attributes: { style: 'footer' } },
{ type: 'tableRow' },
- { type: 'tableCell', attributes: { style: 'data' } },
+ {
+ type: 'tableCell',
+ attributes: {
+ style: 'data',
+ colspan: 2,
+ originalColspan: '2'
+ }
+ },
{ type: 'paragraph', internal: { generated: 'wrapper' } },
'B',
'a',
@@ -811,7 +818,14 @@
{ type: '/tableSection' },
{ type: 'tableSection', attributes: { style: 'body' } },
{ type: 'tableRow' },
- { type: 'tableCell', attributes: { style: 'data' } },
+ {
+ type: 'tableCell',
+ attributes: {
+ style: 'data',
+ rowspan: 2,
+ originalRowspan: '02'
+ }
+ },
{ type: 'paragraph', internal: { generated: 'wrapper' } },
'Q',
'u',
@@ -819,7 +833,14 @@
'x',
{ type: '/paragraph' },
{ type: '/tableCell' },
- { type: 'tableCell', attributes: { style: 'data' } },
+ {
+ type: 'tableCell',
+ attributes: {
+ style: 'data',
+ colspan: NaN,
+ originalColspan: '2 garbage'
+ }
+ },
{ type: 'paragraph', internal: { generated: 'wrapper' } },
'W',
'h',
--
To view, visit https://gerrit.wikimedia.org/r/173443
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic30e0d6f7bfe27c10e505b7c18fe9200f322cfc7
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: wmf/1.25wmf7
Gerrit-Owner: Jforrester <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
