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

idle_test: test_fontlist_key() fails if two font families have the same name #83781

Closed
vstinner opened this issue Feb 10, 2020 · 15 comments
Closed
Assignees
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes tests Tests in the Lib/test dir topic-IDLE

Comments

@vstinner
Copy link
Member

BPO 39600
Nosy @terryjreedy, @vstinner, @taleinat, @serhiy-storchaka, @csabella, @miss-islington
PRs
  • bpo-39600, IDLE: Remove duplicated font names #18430
  • [3.7] bpo-39600, IDLE: Remove duplicated font names (GH-18430) #18436
  • [3.8] bpo-39600, IDLE: Remove duplicated font names (GH-18430) #18437
  • [3.8] bpo-39600, IDLE: Remove duplicated font names (GH-18430) #18441
  • [3.7] bpo-39600, IDLE: Remove duplicated font names (GH-18430) #18442
  • bpo-39600: Adjust code, add idlelib/NEWS item #18449
  • [3.8] bpo-39600: Adjust code, add idlelib/NEWS item (GH-18449) #18450
  • [3.7] bpo-39600: Adjust code, add idlelib/NEWS item (GH-18449) #18451
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2020-02-11.04:24:24.636>
    created_at = <Date 2020-02-10.15:44:03.184>
    labels = ['3.8', 'expert-IDLE', '3.7', 'tests', '3.9']
    title = 'idle_test: test_fontlist_key() fails if two font families have the same name'
    updated_at = <Date 2020-02-11.11:59:49.184>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2020-02-11.11:59:49.184>
    actor = 'vstinner'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2020-02-11.04:24:24.636>
    closer = 'terry.reedy'
    components = ['IDLE', 'Tests']
    creation = <Date 2020-02-10.15:44:03.184>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39600
    keywords = ['patch']
    message_count = 15.0
    messages = ['361700', '361701', '361712', '361721', '361727', '361728', '361729', '361730', '361767', '361769', '361770', '361771', '361772', '361773', '361802']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'vstinner', 'taleinat', 'serhiy.storchaka', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['18430', '18436', '18437', '18441', '18442', '18449', '18450', '18451']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue39600'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @vstinner
    Copy link
    Member Author

    ======================================================================
    FAIL: test_fontlist_key (idlelib.idle_test.test_configdialog.FontPageTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/usr/lib64/python3.7/idlelib/idle_test/test_configdialog.py", line 104, in test_fontlist_key
        self.assertNotEqual(down_font, font)
    AssertionError: 'Cantarell' == 'Cantarell'

    Example:
    ---

    import tkinter.ttk
    import tkinter.font
    frame = tkinter.ttk.Frame()
    families=sorted(tkinter.font.families(frame))
    for family in families:
        print(family)

    Truncated output on my Fedora 31:
    ---
    Abyssinica SIL
    Android Emoji
    AnjaliOldLipi
    Bitstream Vera Sans
    C059
    Caladea
    Cantarell
    Cantarell
    Cantarell
    Cantarell
    (...)
    Comfortaa
    Comfortaa
    (...))
    DejaVu Sans
    DejaVu Sans
    DejaVu Sans
    (...)
    Source Han Serif CN
    Source Han Serif CN
    Source Han Serif CN
    Source Han Serif CN
    Source Han Serif CN
    Source Han Serif CN
    Source Han Serif TW
    Source Han Serif TW
    Source Han Serif TW
    Source Han Serif TW
    Source Han Serif TW
    Source Han Serif TW
    (...)
    ---

    I'm not sure if it's an issue in the unit test or an issue with the widget itself. Does it make sense to display the same font family name twice?

    @vstinner vstinner added the 3.9 only security fixes label Feb 10, 2020
    @vstinner vstinner added topic-IDLE tests Tests in the Lib/test dir 3.9 only security fixes labels Feb 10, 2020
    @vstinner vstinner added topic-IDLE tests Tests in the Lib/test dir labels Feb 10, 2020
    @vstinner vstinner changed the title idle_test: test_fontlist_key() fails if two fonts have the same name idle_test: test_fontlist_key() fails if two font families have the same name Feb 10, 2020
    @vstinner vstinner changed the title idle_test: test_fontlist_key() fails if two fonts have the same name idle_test: test_fontlist_key() fails if two font families have the same name Feb 10, 2020
    @vstinner vstinner added 3.7 (EOL) end of life 3.8 (EOL) end of life labels Feb 10, 2020
    @vstinner
    Copy link
    Member Author

    I proposed PR 18430 to remove duplicated font family names.

    @willingc
    Copy link
    Contributor

    New changeset ed335cf by Victor Stinner in branch 'master':
    bpo-39600, IDLE: Remove duplicated font names (GH-18430)
    ed335cf

    @terryjreedy
    Copy link
    Member

    Serhiy, do you know anything about the tkinter.font.families() tuple having duplicate names? It strikes me as an OS or tk error.

    On Windows, the tuple has groups of related names with a base name, such as 'Segoe UI' both alone and with suffixes, such as 'light', 'black', and 'symbol'. I wonder if on Fedora 31 the duplicates are related names with the suffixes somehow left off. Cheryl, do you see any of the above duplicated names de-duplicated on Ubuntu with suffixes?

    Whether the duplicates are true duplicates, (same family repeated) or mistakes (related families missing suffixes), they should be useless to the user. Clicking or scrolling through the duplicates should not change the font sample as 'Font(family=dupname)' will not change.

    Victor, your example does not create a Listbox, so that cannot be the issue. The only change to ConfigDialog when testing is that it is not made modal. What puzzles me is that test_fontlist_key starts by explicitly activating the first font on the list, so that the first test on the Fedora machine should be 'Android Emoji' != 'Abyssinica SIL'. Perhaps there is a bug in the tk on that machine.

    Carol Willing merged the PR before I could review and edit. I will add a follow-up after getting backport to work.

    @miss-islington
    Copy link
    Contributor

    New changeset 021a569 by Miss Islington (bot) in branch '3.8':
    bpo-39600, IDLE: Remove duplicated font names (GH-18430)
    021a569

    @miss-islington
    Copy link
    Contributor

    New changeset 2e8097d by Miss Islington (bot) in branch '3.7':
    bpo-39600, IDLE: Remove duplicated font names (GH-18430)
    2e8097d

    @vstinner
    Copy link
    Member Author

    In LibreOffice, I see 4 flavors of "Cantarell" with different *weight* :

    Cantarell
    Cantarell Extra Bold
    Cantarell Light
    Cantarell Thin

    whereas tkinter.font.families(frame) returns 4 times the same string: "Cantarell". The C function of Tk is TkpGetFontFamilies().

    The X11 implementation uses XListFonts() which returns a string like "-foundry-family-weight-slant-(...)" but the function only returns the "family" substring: it ignores the foundy and the weight for example. So it's not surprising to get duplicates. Example: "-adobe-helvetica-bold-o-normal--11-80-100-100-p-60-iso8859-1" becomes "helvetica".

    There is also a Xft implementation which uses XftListFonts(). Then it uses XftPatternGetString() to get a font family.

    I see no logic in Tcl to exclude duplicated family names of different weights.

    --

    >>> tkinter.font.names(frame)
    ('TkCaptionFont', 'TkSmallCaptionFont', 'TkTooltipFont', 'TkFixedFont', 'TkHeadingFont', 'TkMenuFont', 'TkIconFont', 'TkTextFont', 'TkDefaultFont')

    That's something different that I would call unrelated ;-)

    @vstinner
    Copy link
    Member Author

    Victor, your example does not create a Listbox, so that cannot be the issue. The only change to ConfigDialog when testing is that it is not made modal. What puzzles me is that test_fontlist_key starts by explicitly activating the first font on the list, so that the first test on the Fedora machine should be 'Android Emoji' != 'Abyssinica SIL'. Perhaps there is a bug in the tk on that machine.

    The test failure comes from our build system where very few fonts are limited. For example, you can imagine that only a few variants of Cantarell are installed.

    Sorry, I forgot to mention that the test failure and the list of strings come from two different machines.

    The list comes from my laptop where I have many fonts installed.

    @csabella
    Copy link
    Contributor

    I don't see the same duplicates that Victor listed, however I do see some duplicates.

    (...)
    DejaVu Sans
    DejaVu Sans Mono
    DejaVu Serif
    (...)
    Manjari
    Manjari
    Manjari
    (...)
    Mukti Narrow
    Mukti Narrow
    (...)
    Ubuntu
    Ubuntu
    Ubuntu Condensed
    Ubuntu Mono
    (...)

    @terryjreedy
    Copy link
    Member

    New changeset 96ce227 by Terry Jan Reedy in branch 'master':
    bpo-39600: Adjust code, add idlelib/NEWS item (GH-18449)
    96ce227

    @terryjreedy
    Copy link
    Member

    font.names() lists 'symbolic' family names guaranteed to exist. 'TkDefaultFont' is the default Text widget font on a particular OS. IDLE replaces that with 'TkFixedFont', which is translated to the actual family name on each OS: Courier on Windows (ugh) and something like Monaco on macOS.

    Thank you for the additional information. For *nix users, removing the useless duplicates should be an improvement. (This is aside from the test issue, which could have been fixed otherwise: perhaps by inserting 'TestFont' at the top of the list and making other adjustments.)

    @terryjreedy
    Copy link
    Member

    Cheryl, it appears that only 'weight' words are deleted. For a given family, Tk only has a (user) settable 'bold', so other weight options must be 'baked in' to the family. Perhaps X11 allows more weight settings within a family, even if not exposed in Tk.

    @miss-islington
    Copy link
    Contributor

    New changeset c372f9b by Miss Islington (bot) in branch '3.8':
    bpo-39600: Adjust code, add idlelib/NEWS item (GH-18449)
    c372f9b

    @miss-islington
    Copy link
    Contributor

    New changeset 32c8840 by Miss Islington (bot) in branch '3.7':
    bpo-39600: Adjust code, add idlelib/NEWS item (GH-18449)
    32c8840

    @vstinner
    Copy link
    Member Author

    Thanks to everybody who was involved in this issue, it's nice to see a test fixed so quickly, especially when it enhances IDLE UI for end users ;-)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes tests Tests in the Lib/test dir topic-IDLE
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants