Item8454: TML2HTML's WYSIWYG_PROTECTED span's contain a spurious br element

Priority:	CurrentState:	AppliesTo:	Component:	WaitingFor:
Normal	No Action Required	Engine	WysiwygPlugin	CrawfordCurrie, PaulHarvey, MichaelTempest, SvenDowideit

<span class="WYSIWYG_PROTECTED"><br />%SEARCH{"legacy" nonoise="on" format="| [[\$topic]] | [[\$wikiname]] |"}%</span>

this doesn't make much sense, nor is there any explanation in the applicable code - so I've removed it. If you know why its needed, or why the \n in the split is needed, please document it.

#SMELL: why put a \n into the split? (at least document it..)
#    my @queue = split( /(\n?%)/s, $text );
#SVEN: I've removed the extra \n capture, and am seeing what the tests tell me..
#      doing so removes an (so far) unexplaind <br /> that was put into the WYSIWYG_PROTECTED bloc..
#      eg. <span class="WYSIWYG_PROTECTED"><br />%TABLESEP%</span>
#SMELL: having removed the \n from the split, the code below now _should_ be rewritten to remove the 
#      other processing of the \n. For now (Feb2010), I've left it as is, so that we can revert to the old 
#      code if/when someone figures out why it was in the split in the first place.
    my @queue = split( /(%)/s, $text );

if this change to the split regex is ok, I'll clean up the rest of the loop code..

-- SvenDowideit - 02 Feb 2010

Added MT, who has more experience than I in wysiwygplugin.

-- PaulHarvey - 02 Feb 2010

I documented why (as far as I can tell) the WYSIWYG_PROTECTED block must contain the leading
in Foswikirev:7432 for Item2254. It is necessary to ensure that macros at the start of a line stay at the start of a line, even when the topic has been through a WYSIWYG editor. This is important for macros that can emit line-oriented TML e.g. %SEARCH{... format="   * $topic"}%.

The TranslatorTests do not show this property, as they preserve newlines. WYSIWYG editors eat newlines, so this only shows up when there is a browser in the loop. I saw this when mucking about with the Selenium-based BrowserTranslatorTests.

Changing to "no action", since I think no further action is required.

-- MichaelTempest - 21 May 2010