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

Bug: 73430
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:
  Esanders: 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 218b2d8..57d69a8 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 !== 1 ) {
-               attributes.colspan = null;
+       if ( attributes.colspan === 1 && Number( attributes.originalColspan ) 
!== 1 ) {
+               spans.colspan = null;
        }
 
-       if ( attributes.rowspan === 1 && attributes.originalRowspan !== 1 ) {
-               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/173363
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: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to