Last modified: 2013-08-22 16:10:10 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T52603, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 50603 - Implement work-arounds for templated table cell rendering issues
Implement work-arounds for templated table cell rendering issues
Status: RESOLVED FIXED
Product: Parsoid
Classification: Unclassified
General (Other open bugs)
unspecified
All All
: High normal
: ---
Assigned To: Gabriel Wicke
:
Depends on:
Blocks: 44498 50366 50589
  Show dependency treegraph
 
Reported: 2013-07-02 19:52 UTC by Gabriel Wicke
Modified: 2013-08-22 16:10 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Gabriel Wicke 2013-07-02 19:52:27 UTC
Eventually we'd like to move towards more self-contained templates, but for now we should at least try to render the content as expected, even if that content remains uneditable for now.

Test cases:

{|
|{{echo|{{!}} Foo}}
|{{echo|style="color: red" {{!}} Bar}}
|}

Foo is bug 50589, while Bar is bug 50366.
Comment 1 Gabriel Wicke 2013-07-02 22:05:36 UTC
Hmm, Bar did not actually work. Fixed test case:

{|
|{{echo|{{!}} Foo}}
|{{nom|Bar}}
|}

With {{nom|bar}} expanding to http://en.wikipedia.org/w/api.php?action=expandtemplates&text={{nom|Bar}}
Comment 2 Gabriel Wicke 2013-08-07 02:02:11 UTC
Some background from https://gerrit.wikimedia.org/r/#/c/76861/:

We are mainly dealing with two cases here (from bug 50603):
1) |{{echo|{{!}} Foo}}
This produces two tds, which can be merged on the DOM just as in the list item case.
2) |{{nom|Bar}} with {{nom|Bar}} expanding to something like 'style="foo"|Bar'
In this case attributes and (at least some) cell content are template-generated. In the templates I have seen the attributes don't contain wikitext links or other problematic syntax that would cause them to parse to non-text content. This means that we can directly convert the prefix of a text node into attributes. The (planned) addition of nested DSR on template content will also give us access to the original wikitext of other template content, which makes this sound even in theoretical edge cases.
In general I am trying to keep parsing context for template expansions as minimal as possible. Basically context-free expansions enable parallel expansion and efficient reuse. In this special case we are bending over backwards to accommodate a specific case of poorly nested template use. In the longer term the goal is to move away from this however, so we should keep the impact of such hacks on our code base as minimal as possible. The implementation should be self-contained enough so that we can disable them once less ideal nestings like these are fixed up in our content, potentially using the information we have in Parsoid itself.
Your assumption that our current parser framework is synchronous is not actually true. Template / extension expansions and link handling are highly asynchronous. Ironically it is exactly this asynchrony and independent expansion that makes handling this case hard. With a synchronous text-based preprocessor pass as in the PHP parser re-tokenization would take care of these cases. The price of this (no parallelism or template reuse, additional complexity of template encapsulation etc) would be high though.
Comment 3 Gerrit Notification Bot 2013-08-16 20:50:11 UTC
Change 79430 had a related patch set uploaded by GWicke:
Bug 50603: Handle |{{echo|{{!}} Foo}}

https://gerrit.wikimedia.org/r/79430
Comment 4 Gerrit Notification Bot 2013-08-16 23:45:03 UTC
Change 79430 merged by jenkins-bot:
Bug 50603, 50589: Handle |{{echo|{{!}} Foo}}

https://gerrit.wikimedia.org/r/79430
Comment 5 Gerrit Notification Bot 2013-08-19 22:31:01 UTC
Change 79943 had a related patch set uploaded by GWicke:
WIP: Bug 50603, 44498: Handle |{{nom|Bar}}

https://gerrit.wikimedia.org/r/79943
Comment 6 Gabriel Wicke 2013-08-20 20:26:55 UTC
Removed dependency on bug 50951 as that is an unrelated VE issue.
Comment 7 Gerrit Notification Bot 2013-08-20 22:34:23 UTC
Change 79943 merged by jenkins-bot:
Bug 50603, 44498: Handle templates straddling cell attributes & content

https://gerrit.wikimedia.org/r/79943
Comment 8 Gabriel Wicke 2013-08-21 00:37:14 UTC
The original test cases are now fixed in master. The second patch will likely be deployed in the next days after round-trip testing is complete.
Comment 9 Gabriel Wicke 2013-08-21 00:40:27 UTC
A somewhat similar issue was found in round-trip testing, and is now tracked in bug 53139.
Comment 10 Gabriel Wicke 2013-08-22 16:10:10 UTC
These work-arounds are now deployed. Closing as fixed.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links