From d1030989bc41d9fcf9b85a1496b99757077024f0 Mon Sep 17 00:00:00 2001 From: Jackmcbarn Date: Thu, 5 Jun 2014 10:57:42 -0400 Subject: [PATCH] Allow passing nils to mw.html Rather than calling error() when nils get passed to mw.html methods, either remove whatever it was that the nil would go to (if that makes sense), or just do nothing. The seemingly inconsistent use of "not x" and "x ~= nil" is to allow any falsey value where it wouldn't be ambiguous (such as class names), but not where it could be (such as attribute values). Bug: 62982 Change-Id: I76773abbb4394aa9bb8c8a08445e019cade3b2bf --- engines/LuaCommon/lualib/mw.html.lua | 56 +++++++++++++------- tests/engines/LuaCommon/HtmlLibraryTests.lua | 44 ++++++++++++--- 2 files changed, 74 insertions(+), 26 deletions(-) diff --git a/engines/LuaCommon/lualib/mw.html.lua b/engines/LuaCommon/lualib/mw.html.lua index 68e2f17f..256fa9e1 100644 --- a/engines/LuaCommon/lualib/mw.html.lua +++ b/engines/LuaCommon/lualib/mw.html.lua @@ -52,13 +52,13 @@ metatable.__tostring = function( t ) return table.concat( ret ) end --- Get an attribute table (name, value) +-- Get an attribute table (name, value) and its index -- -- @param name local function getAttr( t, name ) for i, attr in ipairs( t.attributes ) do if attr.name == name then - return attr + return attr, i end end end @@ -196,7 +196,7 @@ end -- Set an HTML attribute on the node. -- -- @param name Attribute to set, alternative table of name-value pairs --- @param val Value of the attribute +-- @param val Value of the attribute. Nil causes the attribute to be unset methodtable.attr = function( t, name, val ) if type( name ) == 'table' then if val ~= nil then @@ -219,7 +219,7 @@ methodtable.attr = function( t, name, val ) if type( name ) ~= 'string' and type( name ) ~= 'number' then error( 'Invalid name given: The name must be either a string or a number' ) end - if type( val ) ~= 'string' and type( val ) ~= 'number' then + if val ~= nil and type( val ) ~= 'string' and type( val ) ~= 'number' then error( 'Invalid value given: The value must be either a string or a number' ) end @@ -234,10 +234,14 @@ methodtable.attr = function( t, name, val ) error( "Invalid attribute name: " .. name ) end - local attr = getAttr( t, name ) + local attr, i = getAttr( t, name ) if attr then - attr.val = val - else + if val ~= nil then + attr.val = val + else + table.remove( t.attributes, i ) + end + elseif val ~= nil then table.insert( t.attributes, { name = name, val = val } ) end @@ -249,6 +253,10 @@ end -- -- @param class methodtable.addClass = function( t, class ) + if class == nil then + return t + end + if type( class ) ~= 'string' and type( class ) ~= 'number' then error( 'Invalid class given: The name must be either a string or a number' ) end @@ -266,7 +274,7 @@ end -- Set a CSS property to be added to the node's style attribute. -- -- @param name CSS attribute to set, alternative table of name-value pairs --- @param val +-- @param val The value to set. Nil causes it to be unset methodtable.css = function( t, name, val ) if type( name ) == 'table' then if val ~= nil then @@ -289,18 +297,24 @@ methodtable.css = function( t, name, val ) if type( name ) ~= 'string' and type( name ) ~= 'number' then error( 'Invalid CSS given: The name must be either a string or a number' ) end - if type( val ) ~= 'string' and type( val ) ~= 'number' then + if val ~= nil and type( val ) ~= 'string' and type( val ) ~= 'number' then error( 'Invalid CSS given: The value must be either a string or a number' ) end for i, prop in ipairs( t.styles ) do if prop.name == name then - prop.val = val + if val ~= nil then + prop.val = val + else + table.remove( t.styles, i ) + end return t end end - table.insert( t.styles, { name = name, val = val } ) + if val ~= nil then + table.insert( t.styles, { name = name, val = val } ) + end return t end @@ -310,11 +324,11 @@ end -- -- @param css methodtable.cssText = function( t, css ) - if type( css ) ~= 'string' and type( css ) ~= 'number' then - error( 'Invalid CSS given: Must be either a string or a number' ) - end + if css ~= nil then + if type( css ) ~= 'string' and type( css ) ~= 'number' then + error( 'Invalid CSS given: Must be either a string or a number' ) + end - if css then table.insert( t.styles, css ) end return t @@ -341,12 +355,14 @@ end -- @param tagName -- @param args function HtmlBuilder.create( tagName, args ) - if type( tagName ) ~= 'string' then - error( "Tag name must be a string" ) - end + if tagName ~= nil then + if type( tagName ) ~= 'string' then + error( "Tag name must be a string" ) + end - if tagName ~= '' and not isValidTag( tagName ) then - error( "Invalid tag name: " .. tagName ) + if tagName ~= '' and not isValidTag( tagName ) then + error( "Invalid tag name: " .. tagName ) + end end args = args or {} diff --git a/tests/engines/LuaCommon/HtmlLibraryTests.lua b/tests/engines/LuaCommon/HtmlLibraryTests.lua index b6deb742..3c61a5e0 100644 --- a/tests/engines/LuaCommon/HtmlLibraryTests.lua +++ b/tests/engines/LuaCommon/HtmlLibraryTests.lua @@ -55,6 +55,10 @@ local function testAttributeOverride() return getEmptyTestDiv():attr( 'good', 'MediaWiki' ):attr( 'good', 'Wikibase' ) end +local function testAttributeRemoval() + return getEmptyTestDiv():attr( 'a', 'b' ):attr( 'a', nil ) +end + local function testGetAttribute() return getEmptyTestDiv():attr( 'town', 'Berlin' ):getAttr( 'town' ) end @@ -75,8 +79,12 @@ local function testWikitextAppendToSelfClosing() return mw.html.create( 'hr' ):wikitext( 'foo' ) end -local function testEmptyCreate() - return mw.html.create( '' ):wikitext( 'foo' ):tag( 'div' ):attr( 'a', 'b' ):allDone() +local function testCreateWithValue( val ) + return mw.html.create( val ):wikitext( 'foo' ):tag( 'div' ):attr( 'a', 'b' ):allDone() +end + +local function testCssRemoval() + return getEmptyTestDiv():css( 'color', 'red' ):css( 'color', nil ) end local function testComplex() @@ -116,9 +124,6 @@ local tests = { args = { {} }, expect = 'Tag name must be a string' }, - { name = 'mw.html.create (invalid tag 3)', func = mw.html.create, type='ToString', - expect = 'Tag name must be a string' - }, { name = 'mw.html.wikitext', func = testHelper, type='ToString', args = { getEmptyTestDiv(), 'wikitext', 'Plain text' }, expect = { '
Plain text
' } @@ -140,6 +145,10 @@ local tests = { args = { getEmptyTestDiv(), 'attr', 'foo', 'bar' }, expect = { '
' } }, + { name = 'mw.html.attr (nil noop)', func = testHelper, type='ToString', + args = { getEmptyTestDiv(), 'attr', 'foo', nil }, + expect = { '
' } + }, { name = 'mw.html.attr (table 1)', func = testHelper, type='ToString', args = { getEmptyTestDiv(), 'attr', { foo = 'bar' } }, expect = { '
' } @@ -196,6 +205,10 @@ local tests = { args = { getEmptyTestDiv(), 'css', 'foo', 'bar' }, expect = { '
' } }, + { name = 'mw.html.css (nil noop)', func = testHelper, type='ToString', + args = { getEmptyTestDiv(), 'css', 'foo', nil }, + expect = { '
' } + }, { name = 'mw.html.css (invalid name 1)', func = testHelper, type='ToString', args = { getEmptyTestDiv(), 'css', function() end, 'bar' }, expect = 'Invalid CSS given: The name must be either a string or a number' @@ -244,6 +257,14 @@ local tests = { args = { getEmptyTestDiv(), 'cssText', 'mu"ha:-ha"ha' }, expect = { '
' } }, + { name = 'mw.html.addClass (nil)', func = testHelper, type='ToString', + args = { getEmptyTestDiv(), 'addClass' }, + expect = { '
' } + }, + { name = 'mw.html.cssText (nil)', func = testHelper, type='ToString', + args = { getEmptyTestDiv(), 'cssText' }, + expect = { '
' } + }, -- Tests defined above @@ -274,15 +295,26 @@ local tests = { { name = 'mw.html.attr (overrides)', func = testAttributeOverride, type='ToString', expect = { '
' } }, + { name = 'mw.html.attr (removal)', func = testAttributeRemoval, type='ToString', + expect = { '
' } + }, { name = 'mw.html.getAttr', func = testGetAttribute, type='ToString', expect = { 'Berlin' } }, { name = 'mw.html.getAttr (escaping)', func = testGetAttributeEscaping, type='ToString', expect = { '' } }, - { name = 'mw.html.create (empty)', func = testEmptyCreate, type='ToString', + { name = 'mw.html.create (empty string)', func = testCreateWithValue, type='ToString', + args = {''}, expect = { 'foo
' } }, + { name = 'mw.html.create (nil)', func = testCreateWithValue, type='ToString', + args = {nil}, + expect = { 'foo
' } + }, + { name = 'mw.html.css (removal)', func = testCssRemoval, type='ToString', + expect = { '
' } + }, { name = 'mw.html complex test', func = testComplex, type='ToString', expect = { '
' ..