BUG: The font list order of pypdf.annotations.FreeText is different(#1435)

[ssjkamei]

Close #1435

The order of the list was different.

It seems difficult to obtain the actual font size etc. Is it necessary to test?

[stefan6419846]

It seems difficult to obtain the actual font size etc.

Could you please elaborate?

Is it necessary to test?

Yes, in the best case we would check the correct output here as well covering some common combinations.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.43%. Comparing base (96b46ad) to head (423876d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2893   +/-   ##
=======================================
  Coverage   96.43%   96.43%           
=======================================
  Files          52       52           
  Lines        8724     8726    +2     
  Branches     1721     1721           
=======================================
+ Hits         8413     8415    +2     
  Misses        182      182           
  Partials      129      129           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ssjkamei]

It seems difficult to obtain the actual font size etc.

Could you please elaborate?

The problem is that it's difficult for me to capture in code that the font size has changed.
There was originally a test that asked you to see it with your own eyes, but is there any problem with this?

def test_free_text_annotation(pdf_file_path):
    # Arrange
    pdf_path = RESOURCE_ROOT / "crazyones.pdf"
    reader = PdfReader(pdf_path)
    page = reader.pages[0]
    writer = PdfWriter()
    writer.add_page(page)

    # Act
    free_text_annotation = FreeText(
        text="Hello World - bold and italic\nThis is the second line!",
        rect=(50, 550, 200, 650),
        font="Arial",
        bold=True,
        italic=True,
        font_size="20pt",
        font_color="00ff00",
        border_color=None,
        background_color=None,
    )
    writer.add_annotation(0, free_text_annotation)

    free_text_annotation = FreeText(
        text="Another free text annotation (not bold, not italic)",
        rect=(500, 550, 200, 650),
        font="Arial",
        bold=False,
        italic=False,
        font_size="20pt",
        font_color="00ff00",
        border_color="0000ff",
        background_color="cdcdcd",
    )
    writer.add_annotation(0, free_text_annotation)

    # Assert: You need to inspect the file manually
    with open(pdf_file_path, "wb") as fp:
        writer.write(fp)

[ssjkamei]

#2084 can also be closed.

[stefan6419846]

In general, we prefer tests that are automated. Thus, for your change, I would prefer you to add a corresponding test which asserts that we indeed generate the correct font strings from the inputs. IMHO this case can be tested from code and does not need manual inspection.

[ssjkamei]

Is it okay to delete the original test and create a new one?
In this case, as a test, do you mean read it with PDFReader after writing and check the input value as is?
Do you have any hints?

[stefan6419846]

It should not harm to keep the existing test nevertheless.

In my opinion, it would be sufficient to just create different FreeText instances and verify their /DS entry using assert "abc" == FreeText()["/DS"].

[ssjkamei]

Thank you!
I've added this to the existing test, is this correct?

[ssjkamei]

I think #2305 can be closed if there are no problems with the comments.

[stefan6419846]

I've added this to the existing test, is this correct?

Please move this to a dedicated test function, for example test_free_text_annotation__font_specifier or similar.

[ssjkamei]

I changed it. Please help me if there are any problems with the input location.
Sorry for repeating the basic points. Thank you.

[stefan6419846]

Thanks.

[ssjkamei]

@stefan6419846 Please also check the following two issues.

#2084 It can simply be closed.
#2305 Advanced features that allow users to change the internal structure can be closed if that is not what the pypdf requires.