Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed wrong arg order in IMSC attr parsing for cell resolution #402

Merged
merged 2 commits into from Sep 14, 2023

Conversation

chrsan
Copy link
Contributor

@chrsan chrsan commented Sep 12, 2023

This commit fixes the switched order of columns and rows in the IMSC attribute parsing for cellResolution.

Closes #403

@palemieux
Copy link
Contributor

palemieux commented Sep 12, 2023

@chrsan Can you add a unit test at src/test/python/test_imsc_reader.py:

  def test_cell_resolution(self):
    xml_str = """<?xml version="1.0" encoding="UTF-8"?>
    <tt xml:lang="en"
        xmlns="http://www.w3.org/ns/ttml"
        xmlns:ttp="http://www.w3.org/ns/ttml#parameter"
        ttp:cellResolution="32 15">
    </tt>"""
    doc = imsc_reader.to_model(et.ElementTree(et.fromstring(xml_str)))
    self.assertEqual(doc.get_cell_resolution().columns, 32)
    self.assertEqual(doc.get_cell_resolution().rows, 15)

Thanks BTW!

@palemieux palemieux self-requested a review September 12, 2023 15:31
@chrsan
Copy link
Contributor Author

chrsan commented Sep 13, 2023

Thanks for the test case! I've added it in my latest commit.

@chrsan chrsan closed this Sep 13, 2023
@chrsan chrsan reopened this Sep 13, 2023
@chrsan
Copy link
Contributor Author

chrsan commented Sep 13, 2023

@palemieux BTW, I think the initial value for ebutts:linePadding is wrong. It should be 0c and not 1c as it is right now if I'm correct?

@palemieux palemieux merged commit 1a19f33 into sandflow:master Sep 14, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IMSC reader: ttp:cellResolution components are swapped
2 participants