Community
Participate
Working Groups
Build Identifier: 20100204-0846 MediaWiki supports nested tables (don't know about others): http://meta.wikimedia.org/wiki/Help:Table#Nested_tables The WikiText preview tab doesn't display the nested tables correctly, it looks like closing the inner table also closes the outer table. Reproducible: Always Steps to Reproduce: Create a .mediawiki file with the following contents: {| | f( || {| border="1" | a | b |} | , | {| border="1" | c | d |} Switch to the preview pane and select view source from the context menu. Expected: <table><tr><td>f(</td><td><table border="1"><tr><td>a</td><td>b</td></tr></table></td><td>,</td><td><table border="1"><tr><td>a</td><td>b</td></tr></table></td><td></tr></table> Actual: <table><tr><td>f(</td><td>a</td><td>b</td></tr></table><p>| , |</p><table border="1"><tr><td>c</td><td>d</td></tr></table>
Thanks for the bug. Feel free to make a contribution! I've highlighted the relevant code in the attached Mylyn context.
Created attachment 161068 [details] mylyn/context/zip
Very interesting case. I made modification to the table bloc in: bug 381912: [MediaWiki] support for nested blocks in table cells. Now the current output is: <table><tr><td>f(<table border="1"></table></td><td>a</td><td>b</td></tr></table><p>| , |</p><table border="1"><tr><td>c</td><td>d</td></tr></table></body></html> I think, this is because the first table (without border) @canResume()@ when the cell "a" is presented. So every opened child bloc need to close: so the first table with border "1" is closed, and cell "a" belong to the table without border... I am not sure in which order @canResume()@ should be asked. I need to try some stuff and to have a look on how the list bloc works (enclosing list in list is a much more frequent use case).
Here a proposal: https://git.eclipse.org/r/8525 It was about @Block.findCloseOffset(String, int)@: if the current block has a close offset before or at the same position than the nestedParent, it should process the line. Only if the parent block has a close offset before the current block it should close the current block.
Never mind what I wrote in comment #4 ! The nested protocol works well. I made a mistake with the code proposed with bug 381912. I implemented @TableBlock.canResume(String, int)@ and @TableBlock.findCloseOffset(String, int)@ to work on _new line, a new cell or a new header cell_ (see comment in @TableBlock.checkAtNewTableRow(String, int)@). If so, the nested protocol need to start as soon as the @BlockType.TABLE@ is opened (and not like currently when a cell is opened). Otherwise (like in the current implementation) the code is not symmetric. I uploaded a new Patch on Gerrit: https://git.eclipse.org/r/#/c/8525/ I think this solution is the correct one.
Created attachment 223370 [details] JUnit test for MarkupLanguage. To understand the nested protocol, I wrote this test class. I am not sure if this is relevant for the code base. I attached it here if someone is interested.
Created attachment 224649 [details] Screenshot - Second example - Bug 304495 A screenshot of a table defined by following MediaWiki code (second example): bc. {| style="background-color:red;" ! AAA !! AAAAAAAA !! AA |- | a | aaaaa | aaa {| style="background-color:green;" ! B | bbbb |- ! BBB | bb |- ! BBBBB | bb |} |- | aaaa {| style="background-color:yellow;" ! BBBBB !! BBB |- | bbbbb | bbb |- | bb {| style="background-color:blue;" ! CCC !! CCCCC !! CCCCC |- | cc | ccccc | ccc |- | c | cccc | ccc |} | bbbbb |- | bbbb | bb |} | aaaaaaa | aa |- | aaa | aaaaaaaa | aaaa |}
As asked in Gerrit, I added a second test case @testTableNestedMultiple()@. Uploaded patch set 3: https://git.eclipse.org/r/#/c/8525/ You can see a screenshot of this second example in attachment 224649 [details]. I created the expected pattern with using the html code produced by MediaWiki on the same input. Here the process: * Take the HTML code produced by MediaWiki * Format the HTML code with Tidy. * Trim each line (inclusive tabs) * Add \\s* at the end of each line * Put the result in a StringBuilder The code produced by MediaWiki is also compatible with the expected pattern. In my opinion this second case is sufficient for this bug (it covers the requested topics "multi-level nesting" and "tables with multiple columns") There is an example of a malformed markup in the example proposed in comment #0 (I have adapted this example for my first JUnit test case: @testTableNested()@ ). In my opinion, other table issues should be addressed in separate bugs.
(In reply to comment #8) > There is an example of a malformed markup in the example proposed in comment #0 How about this: bc. {| | first table first cell {| | second table first cell |} | first table first cell Notice that the outer table is not closed. Parsing has to be robust - we really want to guard against bad behaviour (such as infinite loops) by adding tests for malformed markup.
In my opinion the "Missing closing mark on the outer table" was already covered in @testTableNested()@. The closing mark is also missing in the example provided by Dan Berindei in comment #0. In oder to by more precise, I fixed the table in @testTableNested()@ (the outer table has now a closing mark) and I added a third case @testTableNestedMalformed()@. I had to correct your example, because for the moment the Wikitext parser do not accept " |" as valid cell opening. This is also the case on a simple table (it has nothing to do with nested table, so I suggest we fix this bug in Bug 396545). Notice that your example is parsed without problem. " |" is interpreted as "starting a pre-bloc" because of the leading space. It could have been a valid solution if MediaWiki had not decided otherwise (the leading blank spaces are ignored before the table marks). There is no infinite loops or stuff like that, it is just that your example required the resolution of this bug and of Bug 396545. In my opinion fixing separate parsing errors should be fixed in separate bugs (even if both deals with tables). In reply to you comment: "better to use assertEquals() since it is much more difficult to analyze a failed pattern match than an equals mismatch": I changed the code @assertContainsPattern(html, pattern);@ for @assertTrue(html.contains(expected));@. My motivation for the pattern was: the proposed pattern was compatible with the current HTML output of MediaWiki... Now it is not, because MediaWiki to not trim all the line as WikiText does... In my opinion, when dealing with HTML code, having white spaces and line breaks near the html tags doesn't matter. But if you prefer the solution with @html.contains(..)@ it is up to you. Uploaded patch set 4: https://git.eclipse.org/r/#/c/8525
Thanks for the contribution Jeremie. I've merged the review, so marking this as fixed.
