Merge "Allow passing nils to mw.html"

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
+		if val ~= nil then
-	else
+			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
+	if css ~= nil then
-		error( 'Invalid CSS given: Must be either a string or a number' )
+		if type( css ) ~= 'string' and type( css ) ~= 'number' then
-	end
+			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
+	if tagName ~= nil then
-		error( "Tag name must be a string" )
+		if type( tagName ) ~= 'string' then
-	end
+			error( "Tag name must be a string" )
+		end
 
-	if tagName ~= '' and not isValidTag( tagName ) then
+		if tagName ~= '' and not isValidTag( tagName ) then
-		error( "Invalid tag name: " .. tagName )
+			error( "Invalid tag name: " .. tagName )
+		end
 	end
 
 	args = args or {}

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()
+local function testCreateWithValue( val )
-	return mw.html.create( '' ):wikitext( 'foo' ):tag( 'div' ):attr( 'a', 'b' ):allDone()
+	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 = { '<div>Plain text</div>' }
@@ -140,6 +145,10 @@ local tests = {
 	  args = { getEmptyTestDiv(), 'attr', 'foo', 'bar' },
 	  expect = { '<div foo="bar"></div>' }
 	},
+	{ name = 'mw.html.attr (nil noop)', func = testHelper, type='ToString',
+	  args = { getEmptyTestDiv(), 'attr', 'foo', nil },
+	  expect = { '<div></div>' }
+	},
 	{ name = 'mw.html.attr (table 1)', func = testHelper, type='ToString',
 	  args = { getEmptyTestDiv(), 'attr', { foo = 'bar' } },
 	  expect = { '<div foo="bar"></div>' }
@@ -196,6 +205,10 @@ local tests = {
 	  args = { getEmptyTestDiv(), 'css', 'foo', 'bar' },
 	  expect = { '<div style="foo:bar;"></div>' }
 	},
+	{ name = 'mw.html.css (nil noop)', func = testHelper, type='ToString',
+	  args = { getEmptyTestDiv(), 'css', 'foo', nil },
+	  expect = { '<div></div>' }
+	},
 	{ 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 = { '<div style="mu&quot;ha:-ha&quot;ha;"></div>' }
 	},
+	{ name = 'mw.html.addClass (nil)', func = testHelper, type='ToString',
+	  args = { getEmptyTestDiv(), 'addClass' },
+	  expect = { '<div></div>' }
+	},
+	{ name = 'mw.html.cssText (nil)', func = testHelper, type='ToString',
+	  args = { getEmptyTestDiv(), 'cssText' },
+	  expect = { '<div></div>' }
+	},
 
 	-- Tests defined above
 
@@ -274,15 +295,26 @@ local tests = {
 	{ name = 'mw.html.attr (overrides)', func = testAttributeOverride, type='ToString',
 	  expect = { '<div good="Wikibase"></div>' }
 	},
+	{ name = 'mw.html.attr (removal)', func = testAttributeRemoval, type='ToString',
+	  expect = { '<div></div>' }
+	},
 	{ name = 'mw.html.getAttr', func = testGetAttribute, type='ToString',
 	  expect = { 'Berlin' }
 	},
 	{ name = 'mw.html.getAttr (escaping)', func = testGetAttributeEscaping, type='ToString',
 	  expect = { '<ble"&rgh>' }
 	},
-	{ name = 'mw.html.create (empty)', func = testEmptyCreate, type='ToString',
+	{ name = 'mw.html.create (empty string)', func = testCreateWithValue, type='ToString',
+	  args = {''},
 	  expect = { 'foo<div a="b"></div>' }
 	},
+	{ name = 'mw.html.create (nil)', func = testCreateWithValue, type='ToString',
+	  args = {nil},
+	  expect = { 'foo<div a="b"></div>' }
+	},
+	{ name = 'mw.html.css (removal)', func = testCssRemoval, type='ToString',
+	  expect = { '<div></div>' }
+	},
 	{ name = 'mw.html complex test', func = testComplex, type='ToString',
 	  expect = {
 		'<div class="firstClass" what="ever"><meh whynot="Русский"><hr a="b" /></meh>' ..