Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1054)

Issue 246940044: fix failing lwml based tests html_* tests, partially fix #369 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 9 months ago by RogerHaase
Modified:
8 years, 9 months ago
Reviewers:
Visibility:
Public.

Description

fix failing lxml based tests html_* tests, partially fix #369

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -19 lines) Patch
M MoinMoin/converter/_tests/test_html_in.py View 2 chunks +8 lines, -6 lines 2 comments Download
M MoinMoin/converter/_tests/test_html_in_out.py View 4 chunks +6 lines, -3 lines 1 comment Download
M MoinMoin/converter/_tests/test_html_out.py View 8 chunks +25 lines, -9 lines 2 comments Download
M MoinMoin/converter/html_in.py View 2 chunks +3 lines, -0 lines 0 comments Download
M MoinMoin/converter/html_out.py View 1 chunk +3 lines, -1 line 1 comment Download

Messages

Total messages: 1
RogerHaase
8 years, 9 months ago (2015-06-12 15:56:02 UTC) #1
https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
File MoinMoin/converter/_tests/test_html_in.py (right):

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
MoinMoin/converter/_tests/test_html_in.py:175:
'/page/body/p/a[text()="Test"][@xlink:href="http:test"]'),
uri is now an invalid scheme

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
MoinMoin/converter/_tests/test_html_in.py:182:
'''/page/body/p[text()="javascript:alert('hi')"]'''),
javascript is an invalid uri and element containing it gets removed. Comment to
explain added to html_in.py

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
File MoinMoin/converter/_tests/test_html_in_out.py (right):

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
MoinMoin/converter/_tests/test_html_in_out.py:206:
'/div/div/table[./thead/tr[td="Header"]][./tfoot/tr[td="Footer"]][./tbody/tr[td="Cell"]]'),
new test is similar to one above, but with tfoot following tbody

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
File MoinMoin/converter/_tests/test_html_out.py (right):

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
MoinMoin/converter/_tests/test_html_out.py:115:
'/span/a[@href="/page.html"][text()="Test"]'),
There is code in html_in.py to support <BASE> tag that converts relative urls
into absolute, see comments added there.

There is no code in html_out to support base tag. Maybe this should be moved to
test_html_in. Similar test on line 191.

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/_tests/tes...
MoinMoin/converter/_tests/test_html_out.py:125:
'/div[@id="a"]/p[@id="c"][text()="Test"]'),
unsure of how to fix this, made a test that passes.

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/html_out.py
File MoinMoin/converter/html_out.py (right):

https://codereview.appspot.com/246940044/diff/1/MoinMoin/converter/html_out.p...
MoinMoin/converter/html_out.py:556: # ckeditor places tfoot before tbody
failing test revealed bug caused by change adding thead, tfoot to moin tables.
Moin parser adds tfoot after tbody, ckeditor adds tfoot before tbody (the html 4
way)
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b