Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[Bug 20897] Ensure htmlText with listStyle=skip roundtrips #6329
Conversation
| set the text of field "TestField" to "one" & CR & "two" & CR & "three" | ||
| set the listStyle of line 1 of field "TestField" to "skip" | ||
| set the listStyle of line 2 of field "TestField" to "disc" | ||
| TestAssert "htmlText roundtrips listStyle beginning with skip", the htmlText of field "TestField" is the htmlText of field "TestField" |
montegoulding
Mar 4, 2018
Member
@livecodepanos I'm not sure I get this test. Isn't the htmlText of the field always going to equal the htmlText of the field?
@livecodepanos I'm not sure I get this test. Isn't the htmlText of the field always going to equal the htmlText of the field?
livecodepanos
Mar 5, 2018
Author
Contributor
Lol, you are right, this was definitely not what I meant to write!
Lol, you are right, this was definitely not what I meant to write!
| @@ -545,6 +551,10 @@ static void export_html_begin_lists(export_html_t& ctxt, uint32_t p_new_style, u | |||
| else | |||
| /* UNCHECKED */ MCStringAppendFormat(ctxt.m_text, "<li value=\"%d\">", p_index); | |||
| } | |||
|
|
|||
| // listStyle = "skip" does need a <li> tag as well | |||
| if (p_new_style == kMCParagraphListStyleSkip) | |||
montegoulding
Mar 4, 2018
Member
@livecodepanos is there a reason not to just drop the p_new_style != kMCParagraphListStyleSkip above?
@livecodepanos is there a reason not to just drop the p_new_style != kMCParagraphListStyleSkip above?
livecodepanos
Mar 6, 2018
Author
Contributor
@montegoulding just dropping p_new_style != kMCParagraphListStyleSkip causes data loss in this use-case:
put "one" & cr & "two" & cr & "three" into field 1
set the listStyle of line 1 of field 1 to "skip"
set the listStyle of line 2 of field 1 to "disc"
set the htmltext of field 1 to the htmltext
as in the first pass ctxt . list_depth equals p_new_depth thus the code in the if block will not be executed, but we do need a <li> tag in this case anyway I think
@montegoulding just dropping p_new_style != kMCParagraphListStyleSkip causes data loss in this use-case:
put "one" & cr & "two" & cr & "three" into field 1
set the listStyle of line 1 of field 1 to "skip"
set the listStyle of line 2 of field 1 to "disc"
set the htmltext of field 1 to the htmltext
as in the first pass ctxt . list_depth equals p_new_depth thus the code in the if block will not be executed, but we do need a <li> tag in this case anyway I think
montegoulding
Mar 6, 2018
Member
hmm... isn't that a general style of list changing but depth remaining the same issue? So style changes we should end the previous list and start a new list?
hmm... isn't that a general style of list changing but depth remaining the same issue? So style changes we should end the previous list and start a new list?
| while(p_new_depth < ctxt . list_depth) | ||
| { | ||
| ctxt . list_depth -= 1; | ||
| MCStringAppendFormat(ctxt.m_text, ctxt.list_styles[ctxt.list_depth] < kMCParagraphListStyleNumeric ? "</ul>" : "</ol>"); | ||
| // listStyle = "skip" is unordered, thus needs </ul> tag | ||
| MCStringAppendFormat(ctxt.m_text, (ctxt.list_styles[ctxt.list_depth] < kMCParagraphListStyleNumeric || ctxt.list_styles[ctxt.list_depth] == kMCParagraphListStyleSkip) ? "</ul>" : "</ol>"); |
montegoulding
Mar 4, 2018
Member
@livecodepanos these are now really long so perhaps a good opportunity to refactor out the ternary operator?
@livecodepanos these are now really long so perhaps a good opportunity to refactor out the ternary operator?
No description provided.